| Message ID | 6fc335ec79f163b7a612af012ce07be6d98f2ef9.1675849634.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/wm: legacy watermark code shuffling | expand |
Hi Jani,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-i915-move-memory-frequency-detection-to-intel_dram-c/20230208-175057
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link: https://lore.kernel.org/r/6fc335ec79f163b7a612af012ce07be6d98f2ef9.1675849634.git.jani.nikula%40intel.com
patch subject: [Intel-gfx] [PATCH 04/10] drm/i915/wm: add .get_hw_state to watermark funcs
config: x86_64-randconfig-a002-20230206 (https://download.01.org/0day-ci/archive/20230208/202302082058.02NjQHJ5-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/3be1dad7406a8c767260601b10af82797025aae3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jani-Nikula/drm-i915-move-memory-frequency-detection-to-intel_dram-c/20230208-175057
git checkout 3be1dad7406a8c767260601b10af82797025aae3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/i9xx_wm.c:3824:6: warning: no previous prototype for function 'ilk_wm_get_hw_state' [-Wmissing-prototypes]
void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
^
drivers/gpu/drm/i915/display/i9xx_wm.c:3824:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
^
static
1 warning generated.
vim +/ilk_wm_get_hw_state +3824 drivers/gpu/drm/i915/display/i9xx_wm.c
f21b5d6fc26440 Jani Nikula 2023-02-08 3823
f21b5d6fc26440 Jani Nikula 2023-02-08 @3824 void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
f21b5d6fc26440 Jani Nikula 2023-02-08 3825 {
f21b5d6fc26440 Jani Nikula 2023-02-08 3826 struct ilk_wm_values *hw = &dev_priv->display.wm.hw;
f21b5d6fc26440 Jani Nikula 2023-02-08 3827 struct intel_crtc *crtc;
f21b5d6fc26440 Jani Nikula 2023-02-08 3828
f21b5d6fc26440 Jani Nikula 2023-02-08 3829 ilk_init_lp_watermarks(dev_priv);
f21b5d6fc26440 Jani Nikula 2023-02-08 3830
f21b5d6fc26440 Jani Nikula 2023-02-08 3831 for_each_intel_crtc(&dev_priv->drm, crtc)
f21b5d6fc26440 Jani Nikula 2023-02-08 3832 ilk_pipe_wm_get_hw_state(crtc);
f21b5d6fc26440 Jani Nikula 2023-02-08 3833
f21b5d6fc26440 Jani Nikula 2023-02-08 3834 hw->wm_lp[0] = intel_uncore_read(&dev_priv->uncore, WM1_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3835 hw->wm_lp[1] = intel_uncore_read(&dev_priv->uncore, WM2_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3836 hw->wm_lp[2] = intel_uncore_read(&dev_priv->uncore, WM3_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3837
f21b5d6fc26440 Jani Nikula 2023-02-08 3838 hw->wm_lp_spr[0] = intel_uncore_read(&dev_priv->uncore, WM1S_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3839 if (DISPLAY_VER(dev_priv) >= 7) {
f21b5d6fc26440 Jani Nikula 2023-02-08 3840 hw->wm_lp_spr[1] = intel_uncore_read(&dev_priv->uncore, WM2S_LP_IVB);
f21b5d6fc26440 Jani Nikula 2023-02-08 3841 hw->wm_lp_spr[2] = intel_uncore_read(&dev_priv->uncore, WM3S_LP_IVB);
f21b5d6fc26440 Jani Nikula 2023-02-08 3842 }
f21b5d6fc26440 Jani Nikula 2023-02-08 3843
f21b5d6fc26440 Jani Nikula 2023-02-08 3844 if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
f21b5d6fc26440 Jani Nikula 2023-02-08 3845 hw->partitioning = (intel_uncore_read(&dev_priv->uncore, WM_MISC) &
f21b5d6fc26440 Jani Nikula 2023-02-08 3846 WM_MISC_DATA_PARTITION_5_6) ?
f21b5d6fc26440 Jani Nikula 2023-02-08 3847 INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
f21b5d6fc26440 Jani Nikula 2023-02-08 3848 else if (IS_IVYBRIDGE(dev_priv))
f21b5d6fc26440 Jani Nikula 2023-02-08 3849 hw->partitioning = (intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL2) &
f21b5d6fc26440 Jani Nikula 2023-02-08 3850 DISP_DATA_PARTITION_5_6) ?
f21b5d6fc26440 Jani Nikula 2023-02-08 3851 INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
f21b5d6fc26440 Jani Nikula 2023-02-08 3852
f21b5d6fc26440 Jani Nikula 2023-02-08 3853 hw->enable_fbc_wm =
f21b5d6fc26440 Jani Nikula 2023-02-08 3854 !(intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL) & DISP_FBC_WM_DIS);
f21b5d6fc26440 Jani Nikula 2023-02-08 3855 }
f21b5d6fc26440 Jani Nikula 2023-02-08 3856
Hi Jani,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-i915-move-memory-frequency-detection-to-intel_dram-c/20230208-175057
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link: https://lore.kernel.org/r/6fc335ec79f163b7a612af012ce07be6d98f2ef9.1675849634.git.jani.nikula%40intel.com
patch subject: [Intel-gfx] [PATCH 04/10] drm/i915/wm: add .get_hw_state to watermark funcs
config: x86_64-randconfig-a013-20230206 (https://download.01.org/0day-ci/archive/20230208/202302082001.KuQFCeRp-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/3be1dad7406a8c767260601b10af82797025aae3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jani-Nikula/drm-i915-move-memory-frequency-detection-to-intel_dram-c/20230208-175057
git checkout 3be1dad7406a8c767260601b10af82797025aae3
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 olddefconfig
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/i9xx_wm.c:3824:6: warning: no previous prototype for 'ilk_wm_get_hw_state' [-Wmissing-prototypes]
3824 | void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
| ^~~~~~~~~~~~~~~~~~~
vim +/ilk_wm_get_hw_state +3824 drivers/gpu/drm/i915/display/i9xx_wm.c
f21b5d6fc26440 Jani Nikula 2023-02-08 3823
f21b5d6fc26440 Jani Nikula 2023-02-08 @3824 void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
f21b5d6fc26440 Jani Nikula 2023-02-08 3825 {
f21b5d6fc26440 Jani Nikula 2023-02-08 3826 struct ilk_wm_values *hw = &dev_priv->display.wm.hw;
f21b5d6fc26440 Jani Nikula 2023-02-08 3827 struct intel_crtc *crtc;
f21b5d6fc26440 Jani Nikula 2023-02-08 3828
f21b5d6fc26440 Jani Nikula 2023-02-08 3829 ilk_init_lp_watermarks(dev_priv);
f21b5d6fc26440 Jani Nikula 2023-02-08 3830
f21b5d6fc26440 Jani Nikula 2023-02-08 3831 for_each_intel_crtc(&dev_priv->drm, crtc)
f21b5d6fc26440 Jani Nikula 2023-02-08 3832 ilk_pipe_wm_get_hw_state(crtc);
f21b5d6fc26440 Jani Nikula 2023-02-08 3833
f21b5d6fc26440 Jani Nikula 2023-02-08 3834 hw->wm_lp[0] = intel_uncore_read(&dev_priv->uncore, WM1_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3835 hw->wm_lp[1] = intel_uncore_read(&dev_priv->uncore, WM2_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3836 hw->wm_lp[2] = intel_uncore_read(&dev_priv->uncore, WM3_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3837
f21b5d6fc26440 Jani Nikula 2023-02-08 3838 hw->wm_lp_spr[0] = intel_uncore_read(&dev_priv->uncore, WM1S_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3839 if (DISPLAY_VER(dev_priv) >= 7) {
f21b5d6fc26440 Jani Nikula 2023-02-08 3840 hw->wm_lp_spr[1] = intel_uncore_read(&dev_priv->uncore, WM2S_LP_IVB);
f21b5d6fc26440 Jani Nikula 2023-02-08 3841 hw->wm_lp_spr[2] = intel_uncore_read(&dev_priv->uncore, WM3S_LP_IVB);
f21b5d6fc26440 Jani Nikula 2023-02-08 3842 }
f21b5d6fc26440 Jani Nikula 2023-02-08 3843
f21b5d6fc26440 Jani Nikula 2023-02-08 3844 if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
f21b5d6fc26440 Jani Nikula 2023-02-08 3845 hw->partitioning = (intel_uncore_read(&dev_priv->uncore, WM_MISC) &
f21b5d6fc26440 Jani Nikula 2023-02-08 3846 WM_MISC_DATA_PARTITION_5_6) ?
f21b5d6fc26440 Jani Nikula 2023-02-08 3847 INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
f21b5d6fc26440 Jani Nikula 2023-02-08 3848 else if (IS_IVYBRIDGE(dev_priv))
f21b5d6fc26440 Jani Nikula 2023-02-08 3849 hw->partitioning = (intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL2) &
f21b5d6fc26440 Jani Nikula 2023-02-08 3850 DISP_DATA_PARTITION_5_6) ?
f21b5d6fc26440 Jani Nikula 2023-02-08 3851 INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
f21b5d6fc26440 Jani Nikula 2023-02-08 3852
f21b5d6fc26440 Jani Nikula 2023-02-08 3853 hw->enable_fbc_wm =
f21b5d6fc26440 Jani Nikula 2023-02-08 3854 !(intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL) & DISP_FBC_WM_DIS);
f21b5d6fc26440 Jani Nikula 2023-02-08 3855 }
f21b5d6fc26440 Jani Nikula 2023-02-08 3856
Hi Jani,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-i915-move-memory-frequency-detection-to-intel_dram-c/20230208-175057
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link: https://lore.kernel.org/r/6fc335ec79f163b7a612af012ce07be6d98f2ef9.1675849634.git.jani.nikula%40intel.com
patch subject: [Intel-gfx] [PATCH 04/10] drm/i915/wm: add .get_hw_state to watermark funcs
config: i386-defconfig (https://download.01.org/0day-ci/archive/20230208/202302082023.fQH3CQDb-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/3be1dad7406a8c767260601b10af82797025aae3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jani-Nikula/drm-i915-move-memory-frequency-detection-to-intel_dram-c/20230208-175057
git checkout 3be1dad7406a8c767260601b10af82797025aae3
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/i9xx_wm.c:3824:6: error: no previous prototype for 'ilk_wm_get_hw_state' [-Werror=missing-prototypes]
3824 | void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
| ^~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/ilk_wm_get_hw_state +3824 drivers/gpu/drm/i915/display/i9xx_wm.c
f21b5d6fc26440 Jani Nikula 2023-02-08 3823
f21b5d6fc26440 Jani Nikula 2023-02-08 @3824 void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
f21b5d6fc26440 Jani Nikula 2023-02-08 3825 {
f21b5d6fc26440 Jani Nikula 2023-02-08 3826 struct ilk_wm_values *hw = &dev_priv->display.wm.hw;
f21b5d6fc26440 Jani Nikula 2023-02-08 3827 struct intel_crtc *crtc;
f21b5d6fc26440 Jani Nikula 2023-02-08 3828
f21b5d6fc26440 Jani Nikula 2023-02-08 3829 ilk_init_lp_watermarks(dev_priv);
f21b5d6fc26440 Jani Nikula 2023-02-08 3830
f21b5d6fc26440 Jani Nikula 2023-02-08 3831 for_each_intel_crtc(&dev_priv->drm, crtc)
f21b5d6fc26440 Jani Nikula 2023-02-08 3832 ilk_pipe_wm_get_hw_state(crtc);
f21b5d6fc26440 Jani Nikula 2023-02-08 3833
f21b5d6fc26440 Jani Nikula 2023-02-08 3834 hw->wm_lp[0] = intel_uncore_read(&dev_priv->uncore, WM1_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3835 hw->wm_lp[1] = intel_uncore_read(&dev_priv->uncore, WM2_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3836 hw->wm_lp[2] = intel_uncore_read(&dev_priv->uncore, WM3_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3837
f21b5d6fc26440 Jani Nikula 2023-02-08 3838 hw->wm_lp_spr[0] = intel_uncore_read(&dev_priv->uncore, WM1S_LP_ILK);
f21b5d6fc26440 Jani Nikula 2023-02-08 3839 if (DISPLAY_VER(dev_priv) >= 7) {
f21b5d6fc26440 Jani Nikula 2023-02-08 3840 hw->wm_lp_spr[1] = intel_uncore_read(&dev_priv->uncore, WM2S_LP_IVB);
f21b5d6fc26440 Jani Nikula 2023-02-08 3841 hw->wm_lp_spr[2] = intel_uncore_read(&dev_priv->uncore, WM3S_LP_IVB);
f21b5d6fc26440 Jani Nikula 2023-02-08 3842 }
f21b5d6fc26440 Jani Nikula 2023-02-08 3843
f21b5d6fc26440 Jani Nikula 2023-02-08 3844 if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
f21b5d6fc26440 Jani Nikula 2023-02-08 3845 hw->partitioning = (intel_uncore_read(&dev_priv->uncore, WM_MISC) &
f21b5d6fc26440 Jani Nikula 2023-02-08 3846 WM_MISC_DATA_PARTITION_5_6) ?
f21b5d6fc26440 Jani Nikula 2023-02-08 3847 INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
f21b5d6fc26440 Jani Nikula 2023-02-08 3848 else if (IS_IVYBRIDGE(dev_priv))
f21b5d6fc26440 Jani Nikula 2023-02-08 3849 hw->partitioning = (intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL2) &
f21b5d6fc26440 Jani Nikula 2023-02-08 3850 DISP_DATA_PARTITION_5_6) ?
f21b5d6fc26440 Jani Nikula 2023-02-08 3851 INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
f21b5d6fc26440 Jani Nikula 2023-02-08 3852
f21b5d6fc26440 Jani Nikula 2023-02-08 3853 hw->enable_fbc_wm =
f21b5d6fc26440 Jani Nikula 2023-02-08 3854 !(intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL) & DISP_FBC_WM_DIS);
f21b5d6fc26440 Jani Nikula 2023-02-08 3855 }
f21b5d6fc26440 Jani Nikula 2023-02-08 3856
On Wed, Feb 08, 2023 at 11:48:42AM +0200, Jani Nikula wrote: > Get rid of the if ladder in intel_modeset_setup_hw_state() and hide a > number of functions by adding a .get_hw_state() hook to watermark > functions. At least for now, combine the platform specific sanitization > to the hw state readouts on the relevant platforms instead of adding a > separate hook for that. > > There's a functional change on PCH split platforms: If i9xx_wm_init() > fails to read plane latency and chooses the nop functions, > ilk_wm_get_hw_state() won't get called for readout. Add the > ilk_init_lp_watermarks() call on that path which now won't be called in > .get_hw_state(), as it looks like the only thing that could make a > difference. That ilk+ nop_funcs stuff is just nonsense. We should just simply nuke it. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/i9xx_wm.c | 24 +++++++++++++++---- > drivers/gpu/drm/i915/display/i9xx_wm.h | 5 ---- > .../gpu/drm/i915/display/intel_display_core.h | 1 + > .../drm/i915/display/intel_modeset_setup.c | 14 ++--------- > drivers/gpu/drm/i915/display/intel_wm.c | 6 +++++ > drivers/gpu/drm/i915/display/intel_wm.h | 1 + > drivers/gpu/drm/i915/display/skl_watermark.c | 11 +++++++-- > drivers/gpu/drm/i915/display/skl_watermark.h | 3 --- > 8 files changed, 39 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c > index 93ad40dad730..889c901aa3e7 100644 > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c > @@ -3505,7 +3505,7 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv, > #undef _FW_WM > #undef _FW_WM_VLV > > -void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) > +static void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) > { > struct g4x_wm_values *wm = &dev_priv->display.wm.g4x; > struct intel_crtc *crtc; > @@ -3598,7 +3598,7 @@ void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) > str_yes_no(wm->fbc_en)); > } > > -void g4x_wm_sanitize(struct drm_i915_private *dev_priv) > +static void g4x_wm_sanitize(struct drm_i915_private *dev_priv) > { > struct intel_plane *plane; > struct intel_crtc *crtc; > @@ -3647,7 +3647,13 @@ void g4x_wm_sanitize(struct drm_i915_private *dev_priv) > mutex_unlock(&dev_priv->display.wm.wm_mutex); > } > > -void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) > +static void g4x_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) > +{ > + g4x_wm_get_hw_state(i915); > + g4x_wm_sanitize(i915); > +} > + > +static void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) > { > struct vlv_wm_values *wm = &dev_priv->display.wm.vlv; > struct intel_crtc *crtc; > @@ -3747,7 +3753,7 @@ void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) > wm->sr.plane, wm->sr.cursor, wm->level, wm->cxsr); > } > > -void vlv_wm_sanitize(struct drm_i915_private *dev_priv) > +static void vlv_wm_sanitize(struct drm_i915_private *dev_priv) > { > struct intel_plane *plane; > struct intel_crtc *crtc; > @@ -3793,6 +3799,12 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv) > mutex_unlock(&dev_priv->display.wm.wm_mutex); > } > > +static void vlv_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) > +{ > + vlv_wm_get_hw_state(i915); > + vlv_wm_sanitize(i915); > +} > + > /* > * FIXME should probably kill this and improve > * the real watermark readout/sanitation instead > @@ -3847,6 +3859,7 @@ static const struct intel_wm_funcs ilk_wm_funcs = { > .compute_intermediate_wm = ilk_compute_intermediate_wm, > .initial_watermarks = ilk_initial_watermarks, > .optimize_watermarks = ilk_optimize_watermarks, > + .get_hw_state = ilk_wm_get_hw_state, > }; > > static const struct intel_wm_funcs vlv_wm_funcs = { > @@ -3855,6 +3868,7 @@ static const struct intel_wm_funcs vlv_wm_funcs = { > .initial_watermarks = vlv_initial_watermarks, > .optimize_watermarks = vlv_optimize_watermarks, > .atomic_update_watermarks = vlv_atomic_update_fifo, > + .get_hw_state = vlv_wm_get_hw_state_and_sanitize, > }; > > static const struct intel_wm_funcs g4x_wm_funcs = { > @@ -3862,6 +3876,7 @@ static const struct intel_wm_funcs g4x_wm_funcs = { > .compute_intermediate_wm = g4x_compute_intermediate_wm, > .initial_watermarks = g4x_initial_watermarks, > .optimize_watermarks = g4x_optimize_watermarks, > + .get_hw_state = g4x_wm_get_hw_state_and_sanitize, > }; > > static const struct intel_wm_funcs pnv_wm_funcs = { > @@ -3895,6 +3910,7 @@ void i9xx_wm_init(struct drm_i915_private *dev_priv) > dev_priv->display.wm.spr_latency[0] && dev_priv->display.wm.cur_latency[0])) { > dev_priv->display.funcs.wm = &ilk_wm_funcs; > } else { > + ilk_init_lp_watermarks(dev_priv); > drm_dbg_kms(&dev_priv->drm, > "Failed to read display plane latency. " > "Disable CxSR\n"); > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.h b/drivers/gpu/drm/i915/display/i9xx_wm.h > index af4721b1909a..e5293a4ff540 100644 > --- a/drivers/gpu/drm/i915/display/i9xx_wm.h > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.h > @@ -13,11 +13,6 @@ struct intel_crtc_state; > struct intel_plane_state; > > int ilk_wm_max_level(const struct drm_i915_private *i915); > -void g4x_wm_get_hw_state(struct drm_i915_private *i915); > -void vlv_wm_get_hw_state(struct drm_i915_private *i915); > -void ilk_wm_get_hw_state(struct drm_i915_private *i915); > -void g4x_wm_sanitize(struct drm_i915_private *i915); > -void vlv_wm_sanitize(struct drm_i915_private *i915); > bool ilk_disable_lp_wm(struct drm_i915_private *i915); > bool intel_set_memory_cxsr(struct drm_i915_private *i915, bool enable); > void i9xx_wm_init(struct drm_i915_private *i915); > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h > index fb8670aa2932..176dbe52025b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_core.h > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h > @@ -85,6 +85,7 @@ struct intel_wm_funcs { > void (*optimize_watermarks)(struct intel_atomic_state *state, > struct intel_crtc *crtc); > int (*compute_global_watermarks)(struct intel_atomic_state *state); > + void (*get_hw_state)(struct drm_i915_private *i915); > }; > > struct intel_audio_state { > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c > index 1cce96146ef5..5359b9663a07 100644 > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c > @@ -25,6 +25,7 @@ > #include "intel_modeset_setup.h" > #include "intel_pch_display.h" > #include "intel_pm.h" > +#include "intel_wm.h" > #include "skl_watermark.h" > > static void intel_crtc_disable_noatomic(struct intel_crtc *crtc, > @@ -724,18 +725,7 @@ void intel_modeset_setup_hw_state(struct drm_i915_private *i915, > > intel_dpll_sanitize_state(i915); > > - if (IS_G4X(i915)) { > - g4x_wm_get_hw_state(i915); > - g4x_wm_sanitize(i915); > - } else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) { > - vlv_wm_get_hw_state(i915); > - vlv_wm_sanitize(i915); > - } else if (DISPLAY_VER(i915) >= 9) { > - skl_wm_get_hw_state(i915); > - skl_wm_sanitize(i915); > - } else if (HAS_PCH_SPLIT(i915)) { > - ilk_wm_get_hw_state(i915); > - } > + intel_wm_get_hw_state(i915); > > for_each_intel_crtc(&i915->drm, crtc) { > struct intel_crtc_state *crtc_state = > diff --git a/drivers/gpu/drm/i915/display/intel_wm.c b/drivers/gpu/drm/i915/display/intel_wm.c > index 15e004bf7eba..97d0fb7e1bbe 100644 > --- a/drivers/gpu/drm/i915/display/intel_wm.c > +++ b/drivers/gpu/drm/i915/display/intel_wm.c > @@ -114,6 +114,12 @@ int intel_compute_global_watermarks(struct intel_atomic_state *state) > return 0; > } > > +void intel_wm_get_hw_state(struct drm_i915_private *i915) > +{ > + if (i915->display.funcs.wm->get_hw_state) > + return i915->display.funcs.wm->get_hw_state(i915); > +} > + > bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state) > { > diff --git a/drivers/gpu/drm/i915/display/intel_wm.h b/drivers/gpu/drm/i915/display/intel_wm.h > index 916302a0077a..b261a6feffca 100644 > --- a/drivers/gpu/drm/i915/display/intel_wm.h > +++ b/drivers/gpu/drm/i915/display/intel_wm.h > @@ -26,6 +26,7 @@ void intel_atomic_update_watermarks(struct intel_atomic_state *state, > void intel_optimize_watermarks(struct intel_atomic_state *state, > struct intel_crtc *crtc); > int intel_compute_global_watermarks(struct intel_atomic_state *state); > +void intel_wm_get_hw_state(struct drm_i915_private *i915); > bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state); > void intel_print_wm_latency(struct drm_i915_private *i915, > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c > index d653217560d3..bb09fdca7161 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -2861,7 +2861,7 @@ static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc, > } > } > > -void skl_wm_get_hw_state(struct drm_i915_private *i915) > +static void skl_wm_get_hw_state(struct drm_i915_private *i915) > { > struct intel_dbuf_state *dbuf_state = > to_intel_dbuf_state(i915->display.dbuf.obj.state); > @@ -2961,7 +2961,7 @@ static bool skl_dbuf_is_misconfigured(struct drm_i915_private *i915) > return false; > } > > -void skl_wm_sanitize(struct drm_i915_private *i915) > +static void skl_wm_sanitize(struct drm_i915_private *i915) > { > struct intel_crtc *crtc; > > @@ -2997,6 +2997,12 @@ void skl_wm_sanitize(struct drm_i915_private *i915) > } > } > > +static void skl_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) > +{ > + skl_wm_get_hw_state(i915); > + skl_wm_sanitize(i915); > +} > + > void intel_wm_state_verify(struct intel_crtc *crtc, > struct intel_crtc_state *new_crtc_state) > { > @@ -3269,6 +3275,7 @@ static void skl_setup_wm_latency(struct drm_i915_private *i915) > > static const struct intel_wm_funcs skl_wm_funcs = { > .compute_global_watermarks = skl_compute_wm, > + .get_hw_state = skl_wm_get_hw_state_and_sanitize, > }; > > void skl_wm_init(struct drm_i915_private *i915) > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h > index 1f81e1a5a4a3..f03fd991b189 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.h > +++ b/drivers/gpu/drm/i915/display/skl_watermark.h > @@ -38,9 +38,6 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, > const struct skl_ddb_entry *entries, > int num_entries, int ignore_idx); > > -void skl_wm_get_hw_state(struct drm_i915_private *i915); > -void skl_wm_sanitize(struct drm_i915_private *i915); > - > void intel_wm_state_verify(struct intel_crtc *crtc, > struct intel_crtc_state *new_crtc_state); > > -- > 2.34.1
On Wed, 08 Feb 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Feb 08, 2023 at 11:48:42AM +0200, Jani Nikula wrote: >> Get rid of the if ladder in intel_modeset_setup_hw_state() and hide a >> number of functions by adding a .get_hw_state() hook to watermark >> functions. At least for now, combine the platform specific sanitization >> to the hw state readouts on the relevant platforms instead of adding a >> separate hook for that. >> >> There's a functional change on PCH split platforms: If i9xx_wm_init() >> fails to read plane latency and chooses the nop functions, >> ilk_wm_get_hw_state() won't get called for readout. Add the >> ilk_init_lp_watermarks() call on that path which now won't be called in >> .get_hw_state(), as it looks like the only thing that could make a >> difference. > > That ilk+ nop_funcs stuff is just nonsense. We should just > simply nuke it. Dates back to 2010 and commit 7f8a85698f5c ("drm/i915: Add the support of memory self-refresh on Ironlake"), with no explanations. The usual. I'll add it on top. I mean remove from the top. Something. BR, Jani. > >> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/display/i9xx_wm.c | 24 +++++++++++++++---- >> drivers/gpu/drm/i915/display/i9xx_wm.h | 5 ---- >> .../gpu/drm/i915/display/intel_display_core.h | 1 + >> .../drm/i915/display/intel_modeset_setup.c | 14 ++--------- >> drivers/gpu/drm/i915/display/intel_wm.c | 6 +++++ >> drivers/gpu/drm/i915/display/intel_wm.h | 1 + >> drivers/gpu/drm/i915/display/skl_watermark.c | 11 +++++++-- >> drivers/gpu/drm/i915/display/skl_watermark.h | 3 --- >> 8 files changed, 39 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c >> index 93ad40dad730..889c901aa3e7 100644 >> --- a/drivers/gpu/drm/i915/display/i9xx_wm.c >> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c >> @@ -3505,7 +3505,7 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv, >> #undef _FW_WM >> #undef _FW_WM_VLV >> >> -void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) >> +static void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) >> { >> struct g4x_wm_values *wm = &dev_priv->display.wm.g4x; >> struct intel_crtc *crtc; >> @@ -3598,7 +3598,7 @@ void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) >> str_yes_no(wm->fbc_en)); >> } >> >> -void g4x_wm_sanitize(struct drm_i915_private *dev_priv) >> +static void g4x_wm_sanitize(struct drm_i915_private *dev_priv) >> { >> struct intel_plane *plane; >> struct intel_crtc *crtc; >> @@ -3647,7 +3647,13 @@ void g4x_wm_sanitize(struct drm_i915_private *dev_priv) >> mutex_unlock(&dev_priv->display.wm.wm_mutex); >> } >> >> -void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) >> +static void g4x_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) >> +{ >> + g4x_wm_get_hw_state(i915); >> + g4x_wm_sanitize(i915); >> +} >> + >> +static void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) >> { >> struct vlv_wm_values *wm = &dev_priv->display.wm.vlv; >> struct intel_crtc *crtc; >> @@ -3747,7 +3753,7 @@ void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) >> wm->sr.plane, wm->sr.cursor, wm->level, wm->cxsr); >> } >> >> -void vlv_wm_sanitize(struct drm_i915_private *dev_priv) >> +static void vlv_wm_sanitize(struct drm_i915_private *dev_priv) >> { >> struct intel_plane *plane; >> struct intel_crtc *crtc; >> @@ -3793,6 +3799,12 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv) >> mutex_unlock(&dev_priv->display.wm.wm_mutex); >> } >> >> +static void vlv_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) >> +{ >> + vlv_wm_get_hw_state(i915); >> + vlv_wm_sanitize(i915); >> +} >> + >> /* >> * FIXME should probably kill this and improve >> * the real watermark readout/sanitation instead >> @@ -3847,6 +3859,7 @@ static const struct intel_wm_funcs ilk_wm_funcs = { >> .compute_intermediate_wm = ilk_compute_intermediate_wm, >> .initial_watermarks = ilk_initial_watermarks, >> .optimize_watermarks = ilk_optimize_watermarks, >> + .get_hw_state = ilk_wm_get_hw_state, >> }; >> >> static const struct intel_wm_funcs vlv_wm_funcs = { >> @@ -3855,6 +3868,7 @@ static const struct intel_wm_funcs vlv_wm_funcs = { >> .initial_watermarks = vlv_initial_watermarks, >> .optimize_watermarks = vlv_optimize_watermarks, >> .atomic_update_watermarks = vlv_atomic_update_fifo, >> + .get_hw_state = vlv_wm_get_hw_state_and_sanitize, >> }; >> >> static const struct intel_wm_funcs g4x_wm_funcs = { >> @@ -3862,6 +3876,7 @@ static const struct intel_wm_funcs g4x_wm_funcs = { >> .compute_intermediate_wm = g4x_compute_intermediate_wm, >> .initial_watermarks = g4x_initial_watermarks, >> .optimize_watermarks = g4x_optimize_watermarks, >> + .get_hw_state = g4x_wm_get_hw_state_and_sanitize, >> }; >> >> static const struct intel_wm_funcs pnv_wm_funcs = { >> @@ -3895,6 +3910,7 @@ void i9xx_wm_init(struct drm_i915_private *dev_priv) >> dev_priv->display.wm.spr_latency[0] && dev_priv->display.wm.cur_latency[0])) { >> dev_priv->display.funcs.wm = &ilk_wm_funcs; >> } else { >> + ilk_init_lp_watermarks(dev_priv); >> drm_dbg_kms(&dev_priv->drm, >> "Failed to read display plane latency. " >> "Disable CxSR\n"); >> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.h b/drivers/gpu/drm/i915/display/i9xx_wm.h >> index af4721b1909a..e5293a4ff540 100644 >> --- a/drivers/gpu/drm/i915/display/i9xx_wm.h >> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.h >> @@ -13,11 +13,6 @@ struct intel_crtc_state; >> struct intel_plane_state; >> >> int ilk_wm_max_level(const struct drm_i915_private *i915); >> -void g4x_wm_get_hw_state(struct drm_i915_private *i915); >> -void vlv_wm_get_hw_state(struct drm_i915_private *i915); >> -void ilk_wm_get_hw_state(struct drm_i915_private *i915); >> -void g4x_wm_sanitize(struct drm_i915_private *i915); >> -void vlv_wm_sanitize(struct drm_i915_private *i915); >> bool ilk_disable_lp_wm(struct drm_i915_private *i915); >> bool intel_set_memory_cxsr(struct drm_i915_private *i915, bool enable); >> void i9xx_wm_init(struct drm_i915_private *i915); >> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h >> index fb8670aa2932..176dbe52025b 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_core.h >> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h >> @@ -85,6 +85,7 @@ struct intel_wm_funcs { >> void (*optimize_watermarks)(struct intel_atomic_state *state, >> struct intel_crtc *crtc); >> int (*compute_global_watermarks)(struct intel_atomic_state *state); >> + void (*get_hw_state)(struct drm_i915_private *i915); >> }; >> >> struct intel_audio_state { >> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c >> index 1cce96146ef5..5359b9663a07 100644 >> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c >> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c >> @@ -25,6 +25,7 @@ >> #include "intel_modeset_setup.h" >> #include "intel_pch_display.h" >> #include "intel_pm.h" >> +#include "intel_wm.h" >> #include "skl_watermark.h" >> >> static void intel_crtc_disable_noatomic(struct intel_crtc *crtc, >> @@ -724,18 +725,7 @@ void intel_modeset_setup_hw_state(struct drm_i915_private *i915, >> >> intel_dpll_sanitize_state(i915); >> >> - if (IS_G4X(i915)) { >> - g4x_wm_get_hw_state(i915); >> - g4x_wm_sanitize(i915); >> - } else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) { >> - vlv_wm_get_hw_state(i915); >> - vlv_wm_sanitize(i915); >> - } else if (DISPLAY_VER(i915) >= 9) { >> - skl_wm_get_hw_state(i915); >> - skl_wm_sanitize(i915); >> - } else if (HAS_PCH_SPLIT(i915)) { >> - ilk_wm_get_hw_state(i915); >> - } >> + intel_wm_get_hw_state(i915); >> >> for_each_intel_crtc(&i915->drm, crtc) { >> struct intel_crtc_state *crtc_state = >> diff --git a/drivers/gpu/drm/i915/display/intel_wm.c b/drivers/gpu/drm/i915/display/intel_wm.c >> index 15e004bf7eba..97d0fb7e1bbe 100644 >> --- a/drivers/gpu/drm/i915/display/intel_wm.c >> +++ b/drivers/gpu/drm/i915/display/intel_wm.c >> @@ -114,6 +114,12 @@ int intel_compute_global_watermarks(struct intel_atomic_state *state) >> return 0; >> } >> >> +void intel_wm_get_hw_state(struct drm_i915_private *i915) >> +{ >> + if (i915->display.funcs.wm->get_hw_state) >> + return i915->display.funcs.wm->get_hw_state(i915); >> +} >> + >> bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state, >> const struct intel_plane_state *plane_state) >> { >> diff --git a/drivers/gpu/drm/i915/display/intel_wm.h b/drivers/gpu/drm/i915/display/intel_wm.h >> index 916302a0077a..b261a6feffca 100644 >> --- a/drivers/gpu/drm/i915/display/intel_wm.h >> +++ b/drivers/gpu/drm/i915/display/intel_wm.h >> @@ -26,6 +26,7 @@ void intel_atomic_update_watermarks(struct intel_atomic_state *state, >> void intel_optimize_watermarks(struct intel_atomic_state *state, >> struct intel_crtc *crtc); >> int intel_compute_global_watermarks(struct intel_atomic_state *state); >> +void intel_wm_get_hw_state(struct drm_i915_private *i915); >> bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state, >> const struct intel_plane_state *plane_state); >> void intel_print_wm_latency(struct drm_i915_private *i915, >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c >> index d653217560d3..bb09fdca7161 100644 >> --- a/drivers/gpu/drm/i915/display/skl_watermark.c >> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c >> @@ -2861,7 +2861,7 @@ static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc, >> } >> } >> >> -void skl_wm_get_hw_state(struct drm_i915_private *i915) >> +static void skl_wm_get_hw_state(struct drm_i915_private *i915) >> { >> struct intel_dbuf_state *dbuf_state = >> to_intel_dbuf_state(i915->display.dbuf.obj.state); >> @@ -2961,7 +2961,7 @@ static bool skl_dbuf_is_misconfigured(struct drm_i915_private *i915) >> return false; >> } >> >> -void skl_wm_sanitize(struct drm_i915_private *i915) >> +static void skl_wm_sanitize(struct drm_i915_private *i915) >> { >> struct intel_crtc *crtc; >> >> @@ -2997,6 +2997,12 @@ void skl_wm_sanitize(struct drm_i915_private *i915) >> } >> } >> >> +static void skl_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) >> +{ >> + skl_wm_get_hw_state(i915); >> + skl_wm_sanitize(i915); >> +} >> + >> void intel_wm_state_verify(struct intel_crtc *crtc, >> struct intel_crtc_state *new_crtc_state) >> { >> @@ -3269,6 +3275,7 @@ static void skl_setup_wm_latency(struct drm_i915_private *i915) >> >> static const struct intel_wm_funcs skl_wm_funcs = { >> .compute_global_watermarks = skl_compute_wm, >> + .get_hw_state = skl_wm_get_hw_state_and_sanitize, >> }; >> >> void skl_wm_init(struct drm_i915_private *i915) >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h >> index 1f81e1a5a4a3..f03fd991b189 100644 >> --- a/drivers/gpu/drm/i915/display/skl_watermark.h >> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h >> @@ -38,9 +38,6 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, >> const struct skl_ddb_entry *entries, >> int num_entries, int ignore_idx); >> >> -void skl_wm_get_hw_state(struct drm_i915_private *i915); >> -void skl_wm_sanitize(struct drm_i915_private *i915); >> - >> void intel_wm_state_verify(struct intel_crtc *crtc, >> struct intel_crtc_state *new_crtc_state); >> >> -- >> 2.34.1
diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c index 93ad40dad730..889c901aa3e7 100644 --- a/drivers/gpu/drm/i915/display/i9xx_wm.c +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c @@ -3505,7 +3505,7 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv, #undef _FW_WM #undef _FW_WM_VLV -void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) +static void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) { struct g4x_wm_values *wm = &dev_priv->display.wm.g4x; struct intel_crtc *crtc; @@ -3598,7 +3598,7 @@ void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv) str_yes_no(wm->fbc_en)); } -void g4x_wm_sanitize(struct drm_i915_private *dev_priv) +static void g4x_wm_sanitize(struct drm_i915_private *dev_priv) { struct intel_plane *plane; struct intel_crtc *crtc; @@ -3647,7 +3647,13 @@ void g4x_wm_sanitize(struct drm_i915_private *dev_priv) mutex_unlock(&dev_priv->display.wm.wm_mutex); } -void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) +static void g4x_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) +{ + g4x_wm_get_hw_state(i915); + g4x_wm_sanitize(i915); +} + +static void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) { struct vlv_wm_values *wm = &dev_priv->display.wm.vlv; struct intel_crtc *crtc; @@ -3747,7 +3753,7 @@ void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv) wm->sr.plane, wm->sr.cursor, wm->level, wm->cxsr); } -void vlv_wm_sanitize(struct drm_i915_private *dev_priv) +static void vlv_wm_sanitize(struct drm_i915_private *dev_priv) { struct intel_plane *plane; struct intel_crtc *crtc; @@ -3793,6 +3799,12 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv) mutex_unlock(&dev_priv->display.wm.wm_mutex); } +static void vlv_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) +{ + vlv_wm_get_hw_state(i915); + vlv_wm_sanitize(i915); +} + /* * FIXME should probably kill this and improve * the real watermark readout/sanitation instead @@ -3847,6 +3859,7 @@ static const struct intel_wm_funcs ilk_wm_funcs = { .compute_intermediate_wm = ilk_compute_intermediate_wm, .initial_watermarks = ilk_initial_watermarks, .optimize_watermarks = ilk_optimize_watermarks, + .get_hw_state = ilk_wm_get_hw_state, }; static const struct intel_wm_funcs vlv_wm_funcs = { @@ -3855,6 +3868,7 @@ static const struct intel_wm_funcs vlv_wm_funcs = { .initial_watermarks = vlv_initial_watermarks, .optimize_watermarks = vlv_optimize_watermarks, .atomic_update_watermarks = vlv_atomic_update_fifo, + .get_hw_state = vlv_wm_get_hw_state_and_sanitize, }; static const struct intel_wm_funcs g4x_wm_funcs = { @@ -3862,6 +3876,7 @@ static const struct intel_wm_funcs g4x_wm_funcs = { .compute_intermediate_wm = g4x_compute_intermediate_wm, .initial_watermarks = g4x_initial_watermarks, .optimize_watermarks = g4x_optimize_watermarks, + .get_hw_state = g4x_wm_get_hw_state_and_sanitize, }; static const struct intel_wm_funcs pnv_wm_funcs = { @@ -3895,6 +3910,7 @@ void i9xx_wm_init(struct drm_i915_private *dev_priv) dev_priv->display.wm.spr_latency[0] && dev_priv->display.wm.cur_latency[0])) { dev_priv->display.funcs.wm = &ilk_wm_funcs; } else { + ilk_init_lp_watermarks(dev_priv); drm_dbg_kms(&dev_priv->drm, "Failed to read display plane latency. " "Disable CxSR\n"); diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.h b/drivers/gpu/drm/i915/display/i9xx_wm.h index af4721b1909a..e5293a4ff540 100644 --- a/drivers/gpu/drm/i915/display/i9xx_wm.h +++ b/drivers/gpu/drm/i915/display/i9xx_wm.h @@ -13,11 +13,6 @@ struct intel_crtc_state; struct intel_plane_state; int ilk_wm_max_level(const struct drm_i915_private *i915); -void g4x_wm_get_hw_state(struct drm_i915_private *i915); -void vlv_wm_get_hw_state(struct drm_i915_private *i915); -void ilk_wm_get_hw_state(struct drm_i915_private *i915); -void g4x_wm_sanitize(struct drm_i915_private *i915); -void vlv_wm_sanitize(struct drm_i915_private *i915); bool ilk_disable_lp_wm(struct drm_i915_private *i915); bool intel_set_memory_cxsr(struct drm_i915_private *i915, bool enable); void i9xx_wm_init(struct drm_i915_private *i915); diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index fb8670aa2932..176dbe52025b 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -85,6 +85,7 @@ struct intel_wm_funcs { void (*optimize_watermarks)(struct intel_atomic_state *state, struct intel_crtc *crtc); int (*compute_global_watermarks)(struct intel_atomic_state *state); + void (*get_hw_state)(struct drm_i915_private *i915); }; struct intel_audio_state { diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c index 1cce96146ef5..5359b9663a07 100644 --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c @@ -25,6 +25,7 @@ #include "intel_modeset_setup.h" #include "intel_pch_display.h" #include "intel_pm.h" +#include "intel_wm.h" #include "skl_watermark.h" static void intel_crtc_disable_noatomic(struct intel_crtc *crtc, @@ -724,18 +725,7 @@ void intel_modeset_setup_hw_state(struct drm_i915_private *i915, intel_dpll_sanitize_state(i915); - if (IS_G4X(i915)) { - g4x_wm_get_hw_state(i915); - g4x_wm_sanitize(i915); - } else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) { - vlv_wm_get_hw_state(i915); - vlv_wm_sanitize(i915); - } else if (DISPLAY_VER(i915) >= 9) { - skl_wm_get_hw_state(i915); - skl_wm_sanitize(i915); - } else if (HAS_PCH_SPLIT(i915)) { - ilk_wm_get_hw_state(i915); - } + intel_wm_get_hw_state(i915); for_each_intel_crtc(&i915->drm, crtc) { struct intel_crtc_state *crtc_state = diff --git a/drivers/gpu/drm/i915/display/intel_wm.c b/drivers/gpu/drm/i915/display/intel_wm.c index 15e004bf7eba..97d0fb7e1bbe 100644 --- a/drivers/gpu/drm/i915/display/intel_wm.c +++ b/drivers/gpu/drm/i915/display/intel_wm.c @@ -114,6 +114,12 @@ int intel_compute_global_watermarks(struct intel_atomic_state *state) return 0; } +void intel_wm_get_hw_state(struct drm_i915_private *i915) +{ + if (i915->display.funcs.wm->get_hw_state) + return i915->display.funcs.wm->get_hw_state(i915); +} + bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { diff --git a/drivers/gpu/drm/i915/display/intel_wm.h b/drivers/gpu/drm/i915/display/intel_wm.h index 916302a0077a..b261a6feffca 100644 --- a/drivers/gpu/drm/i915/display/intel_wm.h +++ b/drivers/gpu/drm/i915/display/intel_wm.h @@ -26,6 +26,7 @@ void intel_atomic_update_watermarks(struct intel_atomic_state *state, void intel_optimize_watermarks(struct intel_atomic_state *state, struct intel_crtc *crtc); int intel_compute_global_watermarks(struct intel_atomic_state *state); +void intel_wm_get_hw_state(struct drm_i915_private *i915); bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state); void intel_print_wm_latency(struct drm_i915_private *i915, diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index d653217560d3..bb09fdca7161 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -2861,7 +2861,7 @@ static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc, } } -void skl_wm_get_hw_state(struct drm_i915_private *i915) +static void skl_wm_get_hw_state(struct drm_i915_private *i915) { struct intel_dbuf_state *dbuf_state = to_intel_dbuf_state(i915->display.dbuf.obj.state); @@ -2961,7 +2961,7 @@ static bool skl_dbuf_is_misconfigured(struct drm_i915_private *i915) return false; } -void skl_wm_sanitize(struct drm_i915_private *i915) +static void skl_wm_sanitize(struct drm_i915_private *i915) { struct intel_crtc *crtc; @@ -2997,6 +2997,12 @@ void skl_wm_sanitize(struct drm_i915_private *i915) } } +static void skl_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915) +{ + skl_wm_get_hw_state(i915); + skl_wm_sanitize(i915); +} + void intel_wm_state_verify(struct intel_crtc *crtc, struct intel_crtc_state *new_crtc_state) { @@ -3269,6 +3275,7 @@ static void skl_setup_wm_latency(struct drm_i915_private *i915) static const struct intel_wm_funcs skl_wm_funcs = { .compute_global_watermarks = skl_compute_wm, + .get_hw_state = skl_wm_get_hw_state_and_sanitize, }; void skl_wm_init(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h index 1f81e1a5a4a3..f03fd991b189 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.h +++ b/drivers/gpu/drm/i915/display/skl_watermark.h @@ -38,9 +38,6 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, const struct skl_ddb_entry *entries, int num_entries, int ignore_idx); -void skl_wm_get_hw_state(struct drm_i915_private *i915); -void skl_wm_sanitize(struct drm_i915_private *i915); - void intel_wm_state_verify(struct intel_crtc *crtc, struct intel_crtc_state *new_crtc_state);
Get rid of the if ladder in intel_modeset_setup_hw_state() and hide a number of functions by adding a .get_hw_state() hook to watermark functions. At least for now, combine the platform specific sanitization to the hw state readouts on the relevant platforms instead of adding a separate hook for that. There's a functional change on PCH split platforms: If i9xx_wm_init() fails to read plane latency and chooses the nop functions, ilk_wm_get_hw_state() won't get called for readout. Add the ilk_init_lp_watermarks() call on that path which now won't be called in .get_hw_state(), as it looks like the only thing that could make a difference. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/i9xx_wm.c | 24 +++++++++++++++---- drivers/gpu/drm/i915/display/i9xx_wm.h | 5 ---- .../gpu/drm/i915/display/intel_display_core.h | 1 + .../drm/i915/display/intel_modeset_setup.c | 14 ++--------- drivers/gpu/drm/i915/display/intel_wm.c | 6 +++++ drivers/gpu/drm/i915/display/intel_wm.h | 1 + drivers/gpu/drm/i915/display/skl_watermark.c | 11 +++++++-- drivers/gpu/drm/i915/display/skl_watermark.h | 3 --- 8 files changed, 39 insertions(+), 26 deletions(-)