From: Jani Nikula <jani.nikula@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 4/7] drm/i915/wm: add .get_hw_state to watermark funcs
Date: Wed, 15 Feb 2023 13:12:57 +0200 [thread overview]
Message-ID: <87ttznqj0m.fsf@intel.com> (raw)
In-Reply-To: <Y+qiPTnHIflK5JOF@intel.com>
On Mon, 13 Feb 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Feb 13, 2023 at 09:59:59PM +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.
>>
>> v2:
>> - Add missing static (kernel test robot)
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks, pushed 1-4 to drm-intel-next to keep things moving.
BR,
Jani.
>
>> ---
>> drivers/gpu/drm/i915/display/i9xx_wm.c | 26 +++++++++++++++----
>> 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, 40 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
>> index 676c79dd7b5a..dfdd40991871 100644
>> --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
>> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
>> @@ -3487,7 +3487,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;
>> @@ -3580,7 +3580,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;
>> @@ -3629,7 +3629,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;
>> @@ -3729,7 +3735,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;
>> @@ -3775,6 +3781,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
>> @@ -3791,7 +3803,7 @@ static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
>> */
>> }
>>
>> -void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
>> +static void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
>> {
>> struct ilk_wm_values *hw = &dev_priv->display.wm.hw;
>> struct intel_crtc *crtc;
>> @@ -3829,6 +3841,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 = {
>> @@ -3837,6 +3850,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 = {
>> @@ -3844,6 +3858,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 = {
>> @@ -3877,6 +3892,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 38e32cce5174..133a3234dea5 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 25d778fb7d15..52614fff0d76 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 bb146124a9ca..c4d14a51869b 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 2b62f818099e..dc582967a25e 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 43bda92d3560..476518201cd5 100644
>> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
>> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
>> @@ -2859,7 +2859,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);
>> @@ -2959,7 +2959,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;
>>
>> @@ -2995,6 +2995,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)
>> {
>> @@ -3272,6 +3278,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
--
Jani Nikula, Intel Open Source Graphics Center
next prev parent reply other threads:[~2023-02-15 11:13 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-13 19:59 [Intel-gfx] [PATCH v2 0/7] drm/i915/wm: legacy watermark code shuffling Jani Nikula
2023-02-13 19:59 ` [Intel-gfx] [PATCH v2 1/7] drm/i915: move memory frequency detection to intel_dram.c Jani Nikula
2023-02-13 20:37 ` Ville Syrjälä
2023-02-13 19:59 ` [Intel-gfx] [PATCH v2 2/7] drm/i915/wm: move remaining watermark code out of intel_pm.c Jani Nikula
2023-02-13 20:41 ` Ville Syrjälä
2023-02-13 19:59 ` [Intel-gfx] [PATCH v2 3/7] drm/i915/wm: move functions to call watermark hooks to intel_wm.[ch] Jani Nikula
2023-02-13 20:42 ` Ville Syrjälä
2023-02-13 19:59 ` [Intel-gfx] [PATCH v2 4/7] drm/i915/wm: add .get_hw_state to watermark funcs Jani Nikula
2023-02-13 20:49 ` Ville Syrjälä
2023-02-15 11:12 ` Jani Nikula [this message]
2023-02-13 20:00 ` [Intel-gfx] [PATCH v2 5/7] drm/i915/wm: move watermark sanitization to intel_wm.[ch] Jani Nikula
2023-02-13 20:53 ` Ville Syrjälä
2023-02-15 11:21 ` Jani Nikula
2023-02-15 12:12 ` Ville Syrjälä
2023-02-15 14:20 ` Jani Nikula
2023-02-13 20:00 ` [Intel-gfx] [PATCH v2 6/7] drm/i915/wm: move watermark debugfs to intel_wm.c Jani Nikula
2023-02-13 20:55 ` Ville Syrjälä
2023-02-13 20:00 ` [Intel-gfx] [PATCH v2 7/7] drm/i915: rename intel_pm_types.h -> display/intel_wm_types.h Jani Nikula
2023-02-13 20:56 ` Ville Syrjälä
2023-02-13 20:37 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/wm: legacy watermark code shuffling (rev2) Patchwork
2023-02-13 20:57 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-02-13 23:57 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ttznqj0m.fsf@intel.com \
--to=jani.nikula@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.