Re: [Intel-gfx] [PATCH v2 07/20] drm/i915: Unify the low level dbuf code

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 8854 bytes --]

>-       /* If 2nd DBuf slice required, enable it here */
>        if (INTEL_GEN(dev_priv) >= 11 && slices_union != hw_enabled_slices)
>-               icl_dbuf_slices_update(dev_priv, slices_union);
>+               gen9_dbuf_slices_update(dev_priv, slices_union);
>}

> static void icl_dbuf_slice_post_update(struct intel_atomic_state *state)
>@@ -15307,9 +15306,8 @@ static void icl_dbuf_slice_post_update(struct intel_atomic_state *state)
>        u8 hw_enabled_slices = dev_priv->enabled_dbuf_slices_mask;
>        u8 required_slices = state->enabled_dbuf_slices_mask;

>-       /* If 2nd DBuf slice is no more required disable it */
>         if (INTEL_GEN(dev_priv) >= 11 && required_slices != hw_enabled_slices)
>-               icl_dbuf_slices_update(dev_priv, required_slices);
>+               gen9_dbuf_slices_update(dev_priv, required_slices);


Doesn't make much sense. Just look - previously we were checking if INTEL_GEN is >= than 11(which _is_ ICL)

and now we still _do_ check if INTEL_GEN is >= 11, but... call now function renamed to gen9


I guess you either need to change INTEL_GEN check to be >=9 to at least look somewhat consistent

or leave it as is. Or at least rename icl_ prefix to gen11_ otherwise that looks inconsistent, i.e

you are now checking that gen is >= 11 and then OK - now let's call gen 9! :)


Stan
________________________________
From: Ville Syrjala <ville.syrjala@linux.intel.com>
Sent: Tuesday, February 25, 2020 7:11:12 PM
To: intel-gfx@lists.freedesktop.org
Cc: Lisovskiy, Stanislav
Subject: [PATCH v2 07/20] drm/i915: Unify the low level dbuf code

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The low level dbuf slice code is rather inconsitent with its
functiona naming and organization. Make it more consistent.

Also share the enable/disable functions between all platforms
since the same code works just fine for all of them.

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  6 +--
 .../drm/i915/display/intel_display_power.c    | 44 ++++++++-----------
 .../drm/i915/display/intel_display_power.h    |  6 +--
 3 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3031e64ee518..6952c398cc43 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15296,9 +15296,8 @@ static void icl_dbuf_slice_pre_update(struct intel_atomic_state *state)
         u8 required_slices = state->enabled_dbuf_slices_mask;
         u8 slices_union = hw_enabled_slices | required_slices;

-       /* If 2nd DBuf slice required, enable it here */
         if (INTEL_GEN(dev_priv) >= 11 && slices_union != hw_enabled_slices)
-               icl_dbuf_slices_update(dev_priv, slices_union);
+               gen9_dbuf_slices_update(dev_priv, slices_union);
 }

 static void icl_dbuf_slice_post_update(struct intel_atomic_state *state)
@@ -15307,9 +15306,8 @@ static void icl_dbuf_slice_post_update(struct intel_atomic_state *state)
         u8 hw_enabled_slices = dev_priv->enabled_dbuf_slices_mask;
         u8 required_slices = state->enabled_dbuf_slices_mask;

-       /* If 2nd DBuf slice is no more required disable it */
         if (INTEL_GEN(dev_priv) >= 11 && required_slices != hw_enabled_slices)
-               icl_dbuf_slices_update(dev_priv, required_slices);
+               gen9_dbuf_slices_update(dev_priv, required_slices);
 }

 static void skl_commit_modeset_enables(struct intel_atomic_state *state)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index e81e561e8ac0..ce3bbc4c7a27 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4433,15 +4433,18 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
         mutex_unlock(&power_domains->lock);
 }

-static void intel_dbuf_slice_set(struct drm_i915_private *dev_priv,
-                                enum dbuf_slice slice, bool enable)
+static void gen9_dbuf_slice_set(struct drm_i915_private *dev_priv,
+                               enum dbuf_slice slice, bool enable)
 {
         i915_reg_t reg = DBUF_CTL_S(slice);
         bool state;
         u32 val;

         val = intel_de_read(dev_priv, reg);
-       val = enable ? (val | DBUF_POWER_REQUEST) : (val & ~DBUF_POWER_REQUEST);
+       if (enable)
+               val |= DBUF_POWER_REQUEST;
+       else
+               val &= ~DBUF_POWER_REQUEST;
         intel_de_write(dev_priv, reg, val);
         intel_de_posting_read(dev_priv, reg);
         udelay(10);
@@ -4452,18 +4455,8 @@ static void intel_dbuf_slice_set(struct drm_i915_private *dev_priv,
                  slice, enable ? "enable" : "disable");
 }

-static void gen9_dbuf_enable(struct drm_i915_private *dev_priv)
-{
-       icl_dbuf_slices_update(dev_priv, BIT(DBUF_S1));
-}
-
-static void gen9_dbuf_disable(struct drm_i915_private *dev_priv)
-{
-       icl_dbuf_slices_update(dev_priv, 0);
-}
-
-void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
-                           u8 req_slices)
+void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv,
+                            u8 req_slices)
 {
         int num_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
         struct i915_power_domains *power_domains = &dev_priv->power_domains;
@@ -4486,28 +4479,29 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
         mutex_lock(&power_domains->lock);

         for (slice = DBUF_S1; slice < num_slices; slice++)
-               intel_dbuf_slice_set(dev_priv, slice,
-                                    req_slices & BIT(slice));
+               gen9_dbuf_slice_set(dev_priv, slice, req_slices & BIT(slice));

         dev_priv->enabled_dbuf_slices_mask = req_slices;

         mutex_unlock(&power_domains->lock);
 }

-static void icl_dbuf_enable(struct drm_i915_private *dev_priv)
+static void gen9_dbuf_enable(struct drm_i915_private *dev_priv)
 {
-       skl_ddb_get_hw_state(dev_priv);
+       dev_priv->enabled_dbuf_slices_mask =
+               intel_enabled_dbuf_slices_mask(dev_priv);
+
         /*
          * Just power up at least 1 slice, we will
          * figure out later which slices we have and what we need.
          */
-       icl_dbuf_slices_update(dev_priv, dev_priv->enabled_dbuf_slices_mask |
-                              BIT(DBUF_S1));
+       gen9_dbuf_slices_update(dev_priv, BIT(DBUF_S1) |
+                               dev_priv->enabled_dbuf_slices_mask);
 }

-static void icl_dbuf_disable(struct drm_i915_private *dev_priv)
+static void gen9_dbuf_disable(struct drm_i915_private *dev_priv)
 {
-       icl_dbuf_slices_update(dev_priv, 0);
+       gen9_dbuf_slices_update(dev_priv, 0);
 }

 static void icl_mbus_init(struct drm_i915_private *dev_priv)
@@ -5067,7 +5061,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
         intel_cdclk_init_hw(dev_priv);

         /* 5. Enable DBUF. */
-       icl_dbuf_enable(dev_priv);
+       gen9_dbuf_enable(dev_priv);

         /* 6. Setup MBUS. */
         icl_mbus_init(dev_priv);
@@ -5090,7 +5084,7 @@ static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
         /* 1. Disable all display engine functions -> aready done */

         /* 2. Disable DBUF */
-       icl_dbuf_disable(dev_priv);
+       gen9_dbuf_disable(dev_priv);

         /* 3. Disable CD clock */
         intel_cdclk_uninit_hw(dev_priv);
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index 601e000ffd0d..1a275611241e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -312,13 +312,13 @@ enum dbuf_slice {
         DBUF_S2,
 };

+void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv,
+                            u8 req_slices);
+
 #define with_intel_display_power(i915, domain, wf) \
         for ((wf) = intel_display_power_get((i915), (domain)); (wf); \
              intel_display_power_put_async((i915), (domain), (wf)), (wf) = 0)

-void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
-                           u8 req_slices);
-
 void chv_phy_powergate_lanes(struct intel_encoder *encoder,
                              bool override, unsigned int mask);
 bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
--
2.24.1


[-- Attachment #1.2: Type: text/html, Size: 22168 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx