* [PATCH 0/3] Enable IPC & WM related WA's
@ 2017-01-31 14:57 Mahesh Kumar
2017-01-31 14:57 ` [PATCH 1/3] drm/i915/bxt: Enable IPC support Mahesh Kumar
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Mahesh Kumar @ 2017-01-31 14:57 UTC (permalink / raw)
To: intel-gfx; +Cc: paulo.r.zanoni, maarten.lankhorst
This series include remaining patches from following series
to enable IPC and Enable/update memory BW related WA's for WM.
https://patchwork.freedesktop.org/series/15562/
Mahesh Kumar (3):
drm/i915/bxt: Enable IPC support
drm/i915: Decode system memory bandwidth
drm/i915/gen9: WM memory bandwidth related workaround
drivers/gpu/drm/i915/i915_drv.c | 184 ++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/i915/i915_drv.h | 27 +++++
drivers/gpu/drm/i915/i915_reg.h | 38 ++++++++
drivers/gpu/drm/i915/intel_display.c | 35 +++++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 179 ++++++++++++++++++++++++++++++----
6 files changed, 444 insertions(+), 20 deletions(-)
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] drm/i915/bxt: Enable IPC support
2017-01-31 14:57 [PATCH 0/3] Enable IPC & WM related WA's Mahesh Kumar
@ 2017-01-31 14:57 ` Mahesh Kumar
2017-01-31 15:56 ` Ander Conselvan De Oliveira
2017-01-31 14:57 ` [PATCH 2/3] drm/i915: Decode system memory bandwidth Mahesh Kumar
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Mahesh Kumar @ 2017-01-31 14:57 UTC (permalink / raw)
To: intel-gfx; +Cc: paulo.r.zanoni, maarten.lankhorst
This patch adds IPC support for platforms. This patch enables IPC
only for BXT/KBL platform as for SKL recommendation is to keep it disabled.
IPC (Isochronous Priority Control) is the hardware feature, which
dynamically controls the memory read priority of Display.
When IPC is enabled, plane read requests are sent at high priority until
filling above the transition watermark, then the requests are sent at
lower priority until dropping below the level 0 watermark.
The lower priority requests allow other memory clients to have better
memory access. When IPC is disabled, all plane read requests are sent at
high priority.
Changes since V1:
- Remove commandline parameter to disable ipc
- Address Paulo's comments
Changes since V2:
- Address review comments
- Set ipc_enabled flag
Changes since V3:
- move ipc_enabled flag assignment inside intel_ipc_enable function
Changes since V4:
- Re-enable IPC after suspend/resume
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 4 +++-
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++++++
5 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ca168b22ee26..5f3b22946971 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1246,7 +1246,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
intel_runtime_pm_enable(dev_priv);
- dev_priv->ipc_enabled = false;
+ intel_enable_ipc(dev_priv);
/* Everything is in place, we can now relax! */
DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
@@ -2439,6 +2439,8 @@ static int intel_runtime_resume(struct device *kdev)
enable_rpm_wakeref_asserts(dev_priv);
+ intel_enable_ipc(dev_priv);
+
if (ret)
DRM_ERROR("Runtime resume failed, disabling it (%d)\n", ret);
else
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 72f9f36ae5ce..36e0a33f876c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6466,6 +6466,7 @@ enum {
#define DISP_FBC_WM_DIS (1<<15)
#define DISP_ARB_CTL2 _MMIO(0x45004)
#define DISP_DATA_PARTITION_5_6 (1<<6)
+#define DISP_IPC_ENABLE (1<<3)
#define DBUF_CTL _MMIO(0x45008)
#define DBUF_POWER_REQUEST (1<<31)
#define DBUF_POWER_STATE (1<<30)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0bf8e1bfbe7e..1aa708b6f55e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -17222,6 +17222,7 @@ void intel_display_resume(struct drm_device *dev)
if (!ret)
ret = __intel_display_resume(dev, state);
+ intel_enable_ipc(dev_priv);
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
mutex_unlock(&dev->mode_config.mutex);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0cec0013ace0..ab7423b0a41b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1787,6 +1787,7 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry **entries,
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
bool ilk_disable_lp_wm(struct drm_device *dev);
int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
+void intel_enable_ipc(struct drm_i915_private *dev_priv);
static inline int intel_enable_rc6(void)
{
return i915.enable_rc6;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 249623d45be0..16e83efa1118 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4667,6 +4667,22 @@ void intel_update_watermarks(struct intel_crtc *crtc)
dev_priv->display.update_wm(crtc);
}
+void intel_enable_ipc(struct drm_i915_private *dev_priv)
+{
+ u32 val;
+
+ dev_priv->ipc_enabled = false;
+ if (!(IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)))
+ return;
+
+ val = I915_READ(DISP_ARB_CTL2);
+
+ val |= DISP_IPC_ENABLE;
+
+ I915_WRITE(DISP_ARB_CTL2, val);
+ dev_priv->ipc_enabled = true;
+}
+
/*
* Lock protecting IPS related data structures
*/
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] drm/i915: Decode system memory bandwidth
2017-01-31 14:57 [PATCH 0/3] Enable IPC & WM related WA's Mahesh Kumar
2017-01-31 14:57 ` [PATCH 1/3] drm/i915/bxt: Enable IPC support Mahesh Kumar
@ 2017-01-31 14:57 ` Mahesh Kumar
2017-01-31 14:57 ` [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround Mahesh Kumar
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Mahesh Kumar @ 2017-01-31 14:57 UTC (permalink / raw)
To: intel-gfx; +Cc: paulo.r.zanoni, maarten.lankhorst
This patch adds support to decode system memory bandwidth
which will be used for arbitrated display memory percentage
calculation in GEN9 based system.
Changes from v1:
- Address comments from Paulo
- implement decode function for SKL/KBL also
Changes from v2:
- Rewrite the code as per HW team inputs
- Addresses review comments
Changes from v3:
- Fix compilation warning
Changes from v4:
- Address review comments
- Round-off the frequency & bandwidth results
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 179 ++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_drv.h | 12 +++
drivers/gpu/drm/i915/i915_reg.h | 37 +++++++++
3 files changed, 228 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5f3b22946971..b8d0dd2811a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -975,6 +975,179 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
DRM_DEBUG_DRIVER("use GPU sempahores? %s\n", yesno(i915.semaphores));
}
+static enum rank
+skl_memdev_get_channel_rank(uint32_t val)
+{
+ uint8_t l_rank, s_rank;
+ uint8_t l_size, s_size;
+
+ if (!val)
+ return I915_DRAM_RANK_INVALID;
+
+ l_size = (val >> SKL_DRAM_SIZE_L_SHIFT) & SKL_DRAM_SIZE_MASK;
+ s_size = (val >> SKL_DRAM_SIZE_S_SHIFT) & SKL_DRAM_SIZE_MASK;
+ l_rank = (val >> SKL_DRAM_RANK_L_SHIFT) & SKL_DRAM_RANK_MASK;
+ s_rank = (val >> SKL_DRAM_RANK_S_SHIFT) & SKL_DRAM_RANK_MASK;
+
+ if ((l_size == 0) && (s_size == 0))
+ return I915_DRAM_RANK_INVALID;
+
+ if (l_rank == SKL_DRAM_RANK_DUAL || s_rank == SKL_DRAM_RANK_DUAL)
+ return I915_DRAM_RANK_DUAL;
+
+ if ((l_size && l_rank == SKL_DRAM_RANK_SINGLE) &&
+ (s_size && s_rank == SKL_DRAM_RANK_SINGLE))
+ return I915_DRAM_RANK_DUAL;
+ return I915_DRAM_RANK_SINGLE;
+}
+
+static int
+skl_get_memdev_info(struct drm_i915_private *dev_priv)
+{
+ struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ uint32_t mem_freq_khz;
+ uint32_t val;
+ enum rank ch0_rank, ch1_rank;
+
+ val = I915_READ(SKL_MC_BIOS_DATA_0_0_0_MCHBAR_PCU);
+ mem_freq_khz = DIV_ROUND_UP((val & SKL_REQ_DATA_MASK) *
+ SKL_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
+
+ val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+ ch0_rank = skl_memdev_get_channel_rank(val);
+ if (ch0_rank != I915_DRAM_RANK_INVALID)
+ memdev_info->num_channels++;
+
+ val = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+ ch1_rank = skl_memdev_get_channel_rank(val);
+ if (ch1_rank != I915_DRAM_RANK_INVALID)
+ memdev_info->num_channels++;
+
+ if (memdev_info->num_channels == 0) {
+ DRM_ERROR("Number of mem channels are zero\n");
+ return -EINVAL;
+ }
+
+ memdev_info->bandwidth_kbps = memdev_info->num_channels *
+ mem_freq_khz * 8;
+
+ if (memdev_info->bandwidth_kbps == 0) {
+ DRM_ERROR("Couldn't get system memory bandwidth\n");
+ return -EINVAL;
+ }
+
+ /*
+ * If any of channel is single rank channel, worst case output will be
+ * same as if single rank memory, so consider single rank memory.
+ */
+ if (ch0_rank == I915_DRAM_RANK_SINGLE
+ || ch1_rank == I915_DRAM_RANK_SINGLE)
+ memdev_info->rank = I915_DRAM_RANK_SINGLE;
+ else
+ memdev_info->rank = max(ch0_rank, ch1_rank);
+
+ if (memdev_info->rank == I915_DRAM_RANK_INVALID) {
+ DRM_ERROR("couldn't get memory rank information\n");
+ return -EINVAL;
+ }
+
+ memdev_info->valid = true;
+ return 0;
+}
+
+static int
+bxt_get_memdev_info(struct drm_i915_private *dev_priv)
+{
+ struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ uint32_t dram_channels;
+ uint32_t mem_freq_khz, val;
+ uint8_t num_active_channels;
+ int i;
+
+ val = I915_READ(BXT_P_CR_MC_BIOS_REQ_0_0_0);
+ mem_freq_khz = DIV_ROUND_UP((val & BXT_REQ_DATA_MASK) *
+ BXT_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
+
+ dram_channels = (val >> BXT_DRAM_CHANNEL_ACTIVE_SHIFT) &
+ BXT_DRAM_CHANNEL_ACTIVE_MASK;
+ num_active_channels = hweight32(dram_channels);
+
+ /* each active bit represents 4-byte channel */
+ memdev_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
+
+ if (memdev_info->bandwidth_kbps == 0) {
+ DRM_ERROR("Couldn't get system memory bandwidth\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Now read each DUNIT8/9/10/11 to check the rank of each dimms.
+ */
+ for (i = 0; i < BXT_D_CR_DRP0_DUNIT_MAX; i++) {
+ val = I915_READ(BXT_D_CR_DRP0_DUNIT(i));
+ if (val != 0xFFFFFFFF) {
+ uint8_t rank;
+ enum rank ch_rank;
+
+ memdev_info->num_channels++;
+ rank = val & BXT_DRAM_RANK_MASK;
+ if (rank == BXT_DRAM_RANK_SINGLE)
+ ch_rank = I915_DRAM_RANK_SINGLE;
+ else if (rank == BXT_DRAM_RANK_DUAL)
+ ch_rank = I915_DRAM_RANK_DUAL;
+ else
+ ch_rank = I915_DRAM_RANK_INVALID;
+
+ /*
+ * If any of channel is single rank channel,
+ * worst case output will be same as if single rank
+ * memory, so consider single rank memory.
+ */
+ if (memdev_info->rank == I915_DRAM_RANK_INVALID)
+ memdev_info->rank = ch_rank;
+ else if (ch_rank == I915_DRAM_RANK_SINGLE)
+ memdev_info->rank = I915_DRAM_RANK_SINGLE;
+ }
+ }
+
+ if (memdev_info->rank == I915_DRAM_RANK_INVALID) {
+ DRM_ERROR("couldn't get memory rank information\n");
+ return -EINVAL;
+ }
+
+ memdev_info->valid = true;
+ return 0;
+}
+
+static void
+intel_get_memdev_info(struct drm_i915_private *dev_priv)
+{
+ struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ int ret;
+
+ memdev_info->valid = false;
+ memdev_info->rank = I915_DRAM_RANK_INVALID;
+ memdev_info->num_channels = 0;
+
+ if (!IS_GEN9(dev_priv))
+ return;
+
+ if (IS_BROXTON(dev_priv))
+ ret = bxt_get_memdev_info(dev_priv);
+ else
+ ret = skl_get_memdev_info(dev_priv);
+ if (ret)
+ return;
+
+ DRM_DEBUG_DRIVER("DRAM bandwidth: %u KBps total-channels: %u\n",
+ memdev_info->bandwidth_kbps,
+ memdev_info->num_channels);
+ DRM_DEBUG_DRIVER("DRAM rank: %s rank\n",
+ (memdev_info->rank == I915_DRAM_RANK_DUAL) ?
+ "dual" : "single");
+}
+
+
/**
* i915_driver_init_hw - setup state requiring device access
* @dev_priv: device private
@@ -1077,6 +1250,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
DRM_DEBUG_DRIVER("can't enable MSI");
}
+ /*
+ * Fill the memdev structure to get the system raw bandwidth
+ * This will be used by WM algorithm, to implement GEN9 based WA
+ */
+ intel_get_memdev_info(dev_priv);
+
return 0;
out_ggtt:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 244628065f94..df24de2b65f2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2357,6 +2357,18 @@ struct drm_i915_private {
bool distrust_bios_wm;
} wm;
+ struct memdev_info {
+ bool valid;
+ uint32_t bandwidth_kbps;
+ uint8_t num_channels;
+ enum rank {
+ I915_DRAM_RANK_INVALID = 0,
+ I915_DRAM_RANK_SINGLE,
+ I915_DRAM_RANK_DUAL
+ } rank;
+ } memdev_info;
+
+
struct i915_runtime_pm pm;
struct {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36e0a33f876c..0f10ab603071 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8063,6 +8063,43 @@ enum {
#define DC_STATE_DEBUG_MASK_CORES (1<<0)
#define DC_STATE_DEBUG_MASK_MEMORY_UP (1<<1)
+#define BXT_P_CR_MC_BIOS_REQ_0_0_0 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x7114)
+#define BXT_REQ_DATA_MASK 0x3F
+#define BXT_DRAM_ACTIVE_CHANNEL_SHIFT 12
+#define BXT_DRAM_ACTIVE_CHANNEL_MASK 0xF
+/*
+ * BIOS programs this field of REQ_DATA [5:0] in integer
+ * multiple of 133333 KHz (133.33MHz)
+ */
+#define BXT_MEMORY_FREQ_MULTIPLIER_HZ 133333333
+#define BXT_D_CR_DRP0_DUNIT8 0x1000
+#define BXT_D_CR_DRP0_DUNIT9 0x1200
+#define BXT_D_CR_DRP0_DUNIT_MAX 4
+#define _MMIO_MCHBAR_DUNIT(x, a, b) _MMIO(MCHBAR_MIRROR_BASE_SNB + (a) + (x)*((b)-(a)))
+#define BXT_D_CR_DRP0_DUNIT(x) _MMIO_MCHBAR_DUNIT(x, BXT_D_CR_DRP0_DUNIT8, BXT_D_CR_DRP0_DUNIT9)
+#define BXT_DRAM_CHANNEL_ACTIVE_SHIFT 12
+#define BXT_DRAM_CHANNEL_ACTIVE_MASK 0xF
+#define BXT_DRAM_RANK_MASK 0x3
+#define BXT_DRAM_RANK_SINGLE 0x1
+#define BXT_DRAM_RANK_DUAL 0x3
+
+/*
+ * SKL memory frequeny multiplier is 266667 KHz (266.67 MHz)
+ */
+#define SKL_MEMORY_FREQ_MULTIPLIER_HZ 266666666
+#define SKL_MC_BIOS_DATA_0_0_0_MCHBAR_PCU _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5E04)
+#define SKL_REQ_DATA_MASK (0xF << 0)
+#define SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x500C)
+#define SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5010)
+#define SKL_DRAM_SIZE_MASK 0x1F
+#define SKL_DRAM_SIZE_L_SHIFT 0
+#define SKL_DRAM_SIZE_S_SHIFT 16
+#define SKL_DRAM_RANK_MASK 0x1
+#define SKL_DRAM_RANK_L_SHIFT 10
+#define SKL_DRAM_RANK_S_SHIFT 26
+#define SKL_DRAM_RANK_SINGLE 0x0
+#define SKL_DRAM_RANK_DUAL 0x1
+
/* Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
* since on HSW we can't write to it using I915_WRITE. */
#define D_COMP_HSW _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5F0C)
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround
2017-01-31 14:57 [PATCH 0/3] Enable IPC & WM related WA's Mahesh Kumar
2017-01-31 14:57 ` [PATCH 1/3] drm/i915/bxt: Enable IPC support Mahesh Kumar
2017-01-31 14:57 ` [PATCH 2/3] drm/i915: Decode system memory bandwidth Mahesh Kumar
@ 2017-01-31 14:57 ` Mahesh Kumar
2017-01-31 17:08 ` kbuild test robot
` (2 more replies)
2017-01-31 15:54 ` ✓ Fi.CI.BAT: success for Enable IPC & WM related WA's Patchwork
2017-02-01 9:54 ` ✗ Fi.CI.BAT: failure for Enable IPC & WM related WA's (rev2) Patchwork
4 siblings, 3 replies; 14+ messages in thread
From: Mahesh Kumar @ 2017-01-31 14:57 UTC (permalink / raw)
To: intel-gfx; +Cc: paulo.r.zanoni, maarten.lankhorst
This patch enables workarounds related to display arbitrated memory
bandwidth only if it's necessary. WA's are applicable for all GEN9
based platforms.
Changes since v1:
- Rebase on top of Paulo's patch series
Changes since v2:
- Address review comments
- Rebase/rework as per other patch changes in series
Changes since v3:
- Rebase the patch
- introduce ww_mutex to protect WM operations
- Protect system memory bandwidth calculation with ww_mutex
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 15 ++++
drivers/gpu/drm/i915/intel_display.c | 34 ++++++++
drivers/gpu/drm/i915/intel_pm.c | 163 +++++++++++++++++++++++++++++++----
4 files changed, 194 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b8d0dd2811a9..a14b2a9d2e6a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -814,6 +814,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
mutex_init(&dev_priv->modeset_restore_lock);
mutex_init(&dev_priv->av_mutex);
mutex_init(&dev_priv->wm.wm_mutex);
+ drm_modeset_lock_init(&dev_priv->wm.wm_ww_mutex);
mutex_init(&dev_priv->pps_mutex);
intel_uc_init_early(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index df24de2b65f2..6e5cdd6c9dfd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1174,6 +1174,13 @@ enum intel_sbi_destination {
SBI_MPHY,
};
+/* SKL+ Watermark arbitrated display bandwidth Workarounds */
+enum watermark_memory_wa {
+ WATERMARK_WA_NONE,
+ WATERMARK_WA_X_TILED,
+ WATERMARK_WA_Y_TILED,
+};
+
#define QUIRK_PIPEA_FORCE (1<<0)
#define QUIRK_LVDS_SSC_DISABLE (1<<1)
#define QUIRK_INVERT_BRIGHTNESS (1<<2)
@@ -1744,8 +1751,15 @@ struct skl_ddb_allocation {
struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
};
+struct skl_mem_bw_attr {
+ enum watermark_memory_wa mem_wa;
+ uint32_t pipe_bw_kbps[I915_MAX_PIPES];
+ bool pipe_y_tiled[I915_MAX_PIPES];
+};
+
struct skl_wm_values {
unsigned dirty_pipes;
+ struct skl_mem_bw_attr mem_attr;
struct skl_ddb_allocation ddb;
};
@@ -2348,6 +2362,7 @@ struct drm_i915_private {
* cstate->wm.need_postvbl_update.
*/
struct mutex wm_mutex;
+ struct drm_modeset_lock wm_ww_mutex; /* protect DDB/WM redistribution among pipes */
/*
* Set during HW readout of watermarks/DDB. Some platforms
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1aa708b6f55e..de512bd8136b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14589,6 +14589,38 @@ static void intel_atomic_track_fbs(struct drm_atomic_state *state)
to_intel_plane(plane)->frontbuffer_bit);
}
+static void
+intel_update_wm_bw_wa(struct drm_atomic_state *state)
+{
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct drm_i915_private *dev_priv = to_i915(state->dev);
+ const struct drm_crtc *crtc;
+ const struct drm_crtc_state *cstate;
+ const struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ struct skl_mem_bw_attr *results = &intel_state->wm_results.mem_attr;
+ struct skl_mem_bw_attr *hw_vals = &dev_priv->wm.skl_hw.mem_attr;
+ int i;
+
+ if (!IS_GEN9(dev_priv))
+ return;
+
+ if (!memdev_info->valid)
+ return;
+
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ hw_vals->pipe_bw_kbps[i] = results->pipe_bw_kbps[i];
+ hw_vals->pipe_y_tiled[i] = results->pipe_y_tiled[i];
+ }
+
+ hw_vals->mem_wa = results->mem_wa;
+
+ /*
+ * As we already updated state variables no need to hold the lock,
+ * other commits can proceed now with their calculation
+ */
+ drm_modeset_unlock(&dev_priv->wm.wm_ww_mutex);
+}
+
/**
* intel_atomic_commit - commit validated state object
* @dev: DRM device
@@ -14629,6 +14661,8 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_shared_dpll_swap_state(state);
intel_atomic_track_fbs(state);
+ intel_update_wm_bw_wa(state);
+
if (intel_state->modeset) {
memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
sizeof(intel_state->min_pixclk));
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 16e83efa1118..ae43df5cdfd8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2887,21 +2887,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
#define SKL_SAGV_BLOCK_TIME 30 /* µs */
-/*
- * FIXME: We still don't have the proper code detect if we need to apply the WA,
- * so assume we'll always need it in order to avoid underruns.
- */
-static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
-{
- struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-
- if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
- IS_KABYLAKE(dev_priv))
- return true;
-
- return false;
-}
-
static bool
intel_has_sagv(struct drm_i915_private *dev_priv)
{
@@ -3049,7 +3034,8 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
latency = dev_priv->wm.skl_latency[level];
- if (skl_needs_memory_bw_wa(intel_state) &&
+ if (intel_state->wm_results.mem_attr.mem_wa !=
+ WATERMARK_WA_NONE &&
plane->base.state->fb->modifier ==
I915_FORMAT_MOD_X_TILED)
latency += 15;
@@ -3581,7 +3567,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
uint32_t y_min_scanlines;
struct intel_atomic_state *state =
to_intel_atomic_state(cstate->base.state);
- bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
+ enum watermark_memory_wa mem_wa;
bool y_tiled, x_tiled;
if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
@@ -3597,7 +3583,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
if (IS_KABYLAKE(dev_priv) && dev_priv->ipc_enabled)
latency += 4;
- if (apply_memory_bw_wa && x_tiled)
+ mem_wa = state->wm_results.mem_attr.mem_wa;
+ if (mem_wa != WATERMARK_WA_NONE && x_tiled)
latency += 15;
width = drm_rect_width(&intel_pstate->base.src) >> 16;
@@ -3632,7 +3619,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
y_min_scanlines = 4;
}
- if (apply_memory_bw_wa)
+ if (mem_wa == WATERMARK_WA_Y_TILED)
y_min_scanlines *= 2;
plane_bytes_per_line = width * cpp;
@@ -4061,6 +4048,16 @@ skl_compute_ddb(struct drm_atomic_state *state)
}
/*
+ * If Watermark workaround is changed we need to recalculate
+ * watermark values for all active pipes
+ */
+ if (intel_state->wm_results.mem_attr.mem_wa !=
+ dev_priv->wm.skl_hw.mem_attr.mem_wa) {
+ realloc_pipes = ~0;
+ intel_state->wm_results.dirty_pipes = ~0;
+ }
+
+ /*
* We're not recomputing for the pipes not included in the commit, so
* make sure we start with the current state.
*/
@@ -4085,6 +4082,130 @@ skl_compute_ddb(struct drm_atomic_state *state)
return 0;
}
+static int
+skl_compute_memory_bandwidth_wm_wa(struct drm_atomic_state *state)
+{
+ struct drm_device *dev = state->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ struct skl_mem_bw_attr *mem_attr = &intel_state->wm_results.mem_attr;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *cstate;
+ int active_pipes = 0;
+ uint32_t max_pipe_bw_kbps = 0, total_pipe_bw_kbps;
+ int display_bw_percentage;
+ bool y_tile_enabled = false;
+ int ret, i;
+
+ if (!IS_GEN9(dev_priv)) {
+ mem_attr->mem_wa = WATERMARK_WA_NONE;
+ return 0;
+ }
+
+ if (!memdev_info->valid)
+ goto exit;
+
+ /*
+ * We hold wm_mutex lock, so any other flip can't proceed beyond WM
+ * calculation step until this flip writes modified WM values.
+ * So it's safe to read plane_state of other pipes without taking CRTC lock
+ */
+ ret = drm_modeset_lock(&dev_priv->wm.wm_ww_mutex, state->acquire_ctx);
+ if (ret)
+ return ret;
+
+ memcpy(mem_attr, &dev_priv->wm.skl_hw.mem_attr, sizeof(mem_attr));
+
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ struct drm_plane *plane;
+ const struct drm_plane_state *pstate;
+ int active_planes = 0;
+ uint32_t max_plane_bw_kbps = 0;
+
+ mem_attr->pipe_y_tiled[i] = false;
+
+ if (!cstate->active) {
+ mem_attr->pipe_bw_kbps[i] = 0;
+ continue;
+ }
+
+ drm_atomic_crtc_state_for_each_plane_state(plane, pstate,
+ cstate) {
+ struct drm_framebuffer *fb;
+ uint32_t plane_bw_kbps;
+ enum plane_id id = to_intel_plane(plane)->id;
+
+ if (!pstate->visible)
+ continue;
+
+ if (WARN_ON(!pstate->fb))
+ continue;
+
+ if (id == PLANE_CURSOR)
+ continue;
+
+ fb = pstate->fb;
+ if ((fb->modifier == I915_FORMAT_MOD_Y_TILED ||
+ fb->modifier == I915_FORMAT_MOD_Yf_TILED))
+ mem_attr->pipe_y_tiled[i] = true;
+
+ plane_bw_kbps = skl_adjusted_plane_pixel_rate(
+ to_intel_crtc_state(cstate),
+ to_intel_plane_state(pstate));
+ max_plane_bw_kbps = max(plane_bw_kbps,
+ max_plane_bw_kbps);
+ active_planes++;
+ }
+ mem_attr->pipe_bw_kbps[i] = max_plane_bw_kbps * active_planes;
+ }
+
+ for_each_pipe(dev_priv, i) {
+ if (mem_attr->pipe_bw_kbps[i]) {
+ max_pipe_bw_kbps = max(max_pipe_bw_kbps,
+ mem_attr->pipe_bw_kbps[i]);
+ active_pipes++;
+ }
+ if (mem_attr->pipe_y_tiled[i])
+ y_tile_enabled = true;
+ }
+
+ total_pipe_bw_kbps = max_pipe_bw_kbps * active_pipes;
+ display_bw_percentage = DIV_ROUND_UP_ULL((uint64_t)total_pipe_bw_kbps *
+ 100, memdev_info->bandwidth_kbps);
+
+ /*
+ * If there is any Ytile plane enabled and arbitrated display
+ * bandwidth > 20% of raw system memory bandwidth
+ * Enale Y-tile related WA
+ *
+ * If memory is dual channel single rank, Xtile limit = 35%, else Xtile
+ * limit = 60%
+ * If there is no Ytile plane enabled and
+ * arbitrated display bandwidth > Xtile limit
+ * Enable X-tile realated WA
+ */
+ if (y_tile_enabled && (display_bw_percentage > 20))
+ mem_attr->mem_wa = WATERMARK_WA_Y_TILED;
+ else {
+ int x_tile_percentage = 60;
+ enum rank rank = memdev_info->rank;
+
+ if ((rank == I915_DRAM_RANK_SINGLE) &&
+ (memdev_info->num_channels == 2))
+ x_tile_percentage = 35;
+
+ if (display_bw_percentage > x_tile_percentage)
+ mem_attr->mem_wa = WATERMARK_WA_X_TILED;
+ }
+
+ return 0;
+
+exit:
+ mem_attr->mem_wa = WATERMARK_WA_Y_TILED;
+ return 0;
+}
+
static void
skl_copy_wm_for_pipe(struct skl_wm_values *dst,
struct skl_wm_values *src,
@@ -4160,6 +4281,10 @@ skl_compute_wm(struct drm_atomic_state *state)
/* Clear all dirty flags */
results->dirty_pipes = 0;
+ ret = skl_compute_memory_bandwidth_wm_wa(state);
+ if (ret)
+ return ret;
+
ret = skl_compute_ddb(state);
if (ret)
return ret;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for Enable IPC & WM related WA's
2017-01-31 14:57 [PATCH 0/3] Enable IPC & WM related WA's Mahesh Kumar
` (2 preceding siblings ...)
2017-01-31 14:57 ` [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround Mahesh Kumar
@ 2017-01-31 15:54 ` Patchwork
2017-02-01 9:54 ` ✗ Fi.CI.BAT: failure for Enable IPC & WM related WA's (rev2) Patchwork
4 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-01-31 15:54 UTC (permalink / raw)
To: Kumar, Mahesh; +Cc: intel-gfx
== Series Details ==
Series: Enable IPC & WM related WA's
URL : https://patchwork.freedesktop.org/series/18842/
State : success
== Summary ==
Series 18842v1 Enable IPC & WM related WA's
https://patchwork.freedesktop.org/api/1.0/series/18842/revisions/1/mbox/
fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14
fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39
fi-bxt-j4205 total:247 pass:225 dwarn:0 dfail:0 fail:0 skip:22
fi-bxt-t5700 total:78 pass:65 dwarn:0 dfail:0 fail:0 skip:12
fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-kbl-7500u total:247 pass:224 dwarn:0 dfail:0 fail:2 skip:21
fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20
fi-skl-6700k total:247 pass:222 dwarn:4 dfail:0 fail:0 skip:21
fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32
a0695bf4e1c12de4863e775747fe850b92661dc6 drm-tip: 2017y-01m-31d-13h-38m-55s UTC integration manifest
7876fae drm/i915: Decode system memory bandwidth
41469f0 drm/i915/bxt: Enable IPC support
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3655/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/i915/bxt: Enable IPC support
2017-01-31 14:57 ` [PATCH 1/3] drm/i915/bxt: Enable IPC support Mahesh Kumar
@ 2017-01-31 15:56 ` Ander Conselvan De Oliveira
2017-02-01 6:44 ` Mahesh Kumar
0 siblings, 1 reply; 14+ messages in thread
From: Ander Conselvan De Oliveira @ 2017-01-31 15:56 UTC (permalink / raw)
To: Mahesh Kumar, intel-gfx; +Cc: paulo.r.zanoni, maarten.lankhorst
On Tue, 2017-01-31 at 20:27 +0530, Mahesh Kumar wrote:
> This patch adds IPC support for platforms. This patch enables IPC
> only for BXT/KBL platform as for SKL recommendation is to keep it disabled.
> IPC (Isochronous Priority Control) is the hardware feature, which
> dynamically controls the memory read priority of Display.
>
> When IPC is enabled, plane read requests are sent at high priority until
> filling above the transition watermark, then the requests are sent at
> lower priority until dropping below the level 0 watermark.
> The lower priority requests allow other memory clients to have better
> memory access. When IPC is disabled, all plane read requests are sent at
> high priority.
>
> Changes since V1:
> - Remove commandline parameter to disable ipc
> - Address Paulo's comments
> Changes since V2:
> - Address review comments
> - Set ipc_enabled flag
> Changes since V3:
> - move ipc_enabled flag assignment inside intel_ipc_enable function
> Changes since V4:
> - Re-enable IPC after suspend/resume
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 4 +++-
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++++++
> 5 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ca168b22ee26..5f3b22946971 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1246,7 +1246,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct
> pci_device_id *ent)
>
> intel_runtime_pm_enable(dev_priv);
>
> - dev_priv->ipc_enabled = false;
> + intel_enable_ipc(dev_priv);
>
> /* Everything is in place, we can now relax! */
> DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
> @@ -2439,6 +2439,8 @@ static int intel_runtime_resume(struct device *kdev)
>
> enable_rpm_wakeref_asserts(dev_priv);
>
> + intel_enable_ipc(dev_priv);
> +
> if (ret)
> DRM_ERROR("Runtime resume failed, disabling it (%d)\n", ret);
> else
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 72f9f36ae5ce..36e0a33f876c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6466,6 +6466,7 @@ enum {
> #define DISP_FBC_WM_DIS (1<<15)
> #define DISP_ARB_CTL2 _MMIO(0x45004)
> #define DISP_DATA_PARTITION_5_6 (1<<6)
> +#define DISP_IPC_ENABLE (1<<3)
> #define DBUF_CTL _MMIO(0x45008)
> #define DBUF_POWER_REQUEST (1<<31)
> #define DBUF_POWER_STATE (1<<30)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 0bf8e1bfbe7e..1aa708b6f55e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -17222,6 +17222,7 @@ void intel_display_resume(struct drm_device *dev)
> if (!ret)
> ret = __intel_display_resume(dev, state);
>
> + intel_enable_ipc(dev_priv);
> drm_modeset_drop_locks(&ctx);
> drm_modeset_acquire_fini(&ctx);
> mutex_unlock(&dev->mode_config.mutex);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 0cec0013ace0..ab7423b0a41b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1787,6 +1787,7 @@ bool skl_ddb_allocation_overlaps(const struct
> skl_ddb_entry **entries,
> uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
> bool ilk_disable_lp_wm(struct drm_device *dev);
> int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
> +void intel_enable_ipc(struct drm_i915_private *dev_priv);
> static inline int intel_enable_rc6(void)
> {
> return i915.enable_rc6;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 249623d45be0..16e83efa1118 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4667,6 +4667,22 @@ void intel_update_watermarks(struct intel_crtc *crtc)
> dev_priv->display.update_wm(crtc);
> }
>
> +void intel_enable_ipc(struct drm_i915_private *dev_priv)
> +{
> + u32 val;
> +
> + dev_priv->ipc_enabled = false;
> + if (!(IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)))
> + return;
Do we want this enabled in Geminilake?
Ander
> +
> + val = I915_READ(DISP_ARB_CTL2);
> +
> + val |= DISP_IPC_ENABLE;
> +
> + I915_WRITE(DISP_ARB_CTL2, val);
> + dev_priv->ipc_enabled = true;
> +}
> +
> /*
> * Lock protecting IPS related data structures
> */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround
2017-01-31 14:57 ` [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround Mahesh Kumar
@ 2017-01-31 17:08 ` kbuild test robot
2017-01-31 18:58 ` kbuild test robot
2017-02-01 9:55 ` Maarten Lankhorst
2 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2017-01-31 17:08 UTC (permalink / raw)
To: Mahesh Kumar; +Cc: intel-gfx, kbuild-all, maarten.lankhorst, paulo.r.zanoni
[-- Attachment #1: Type: text/plain, Size: 2119 bytes --]
Hi Mahesh,
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20170130]
[cannot apply to v4.10-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Mahesh-Kumar/Enable-IPC-WM-related-WA-s/20170131-230708
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/intel_pm.c: In function 'skl_compute_memory_bandwidth_wm_wa':
>> drivers/gpu/drm/i915/intel_pm.c:4118:56: warning: argument to 'sizeof' in 'memcpy' call is the same expression as the destination; did you mean to dereference it? [-Wsizeof-pointer-memaccess]
memcpy(mem_attr, &dev_priv->wm.skl_hw.mem_attr, sizeof(mem_attr));
^
vim +4118 drivers/gpu/drm/i915/intel_pm.c
4102 mem_attr->mem_wa = WATERMARK_WA_NONE;
4103 return 0;
4104 }
4105
4106 if (!memdev_info->valid)
4107 goto exit;
4108
4109 /*
4110 * We hold wm_mutex lock, so any other flip can't proceed beyond WM
4111 * calculation step until this flip writes modified WM values.
4112 * So it's safe to read plane_state of other pipes without taking CRTC lock
4113 */
4114 ret = drm_modeset_lock(&dev_priv->wm.wm_ww_mutex, state->acquire_ctx);
4115 if (ret)
4116 return ret;
4117
> 4118 memcpy(mem_attr, &dev_priv->wm.skl_hw.mem_attr, sizeof(mem_attr));
4119
4120 for_each_crtc_in_state(state, crtc, cstate, i) {
4121 struct drm_plane *plane;
4122 const struct drm_plane_state *pstate;
4123 int active_planes = 0;
4124 uint32_t max_plane_bw_kbps = 0;
4125
4126 mem_attr->pipe_y_tiled[i] = false;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38329 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround
2017-01-31 14:57 ` [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround Mahesh Kumar
2017-01-31 17:08 ` kbuild test robot
@ 2017-01-31 18:58 ` kbuild test robot
2017-02-01 9:24 ` Mahesh Kumar
2017-02-01 9:55 ` Maarten Lankhorst
2 siblings, 1 reply; 14+ messages in thread
From: kbuild test robot @ 2017-01-31 18:58 UTC (permalink / raw)
To: Mahesh Kumar; +Cc: intel-gfx, kbuild-all, maarten.lankhorst, paulo.r.zanoni
[-- Attachment #1: Type: text/plain, Size: 2881 bytes --]
Hi Mahesh,
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20170130]
[cannot apply to v4.10-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Mahesh-Kumar/Enable-IPC-WM-related-WA-s/20170131-230708
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from arch/x86/include/asm/string.h:2:0,
from include/linux/string.h:18,
from arch/x86/include/asm/page_32.h:34,
from arch/x86/include/asm/page.h:13,
from arch/x86/include/asm/processor.h:17,
from include/linux/mutex.h:19,
from include/linux/notifier.h:13,
from include/linux/clk.h:17,
from include/linux/cpufreq.h:14,
from drivers/gpu/drm/i915/intel_pm.c:28:
drivers/gpu/drm/i915/intel_pm.c: In function 'skl_compute_memory_bandwidth_wm_wa':
>> drivers/gpu/drm/i915/intel_pm.c:4118:56: warning: argument to 'sizeof' in '__builtin_memcpy' call is the same expression as the destination; did you mean to dereference it? [-Wsizeof-pointer-memaccess]
memcpy(mem_attr, &dev_priv->wm.skl_hw.mem_attr, sizeof(mem_attr));
^
arch/x86/include/asm/string_32.h:182:48: note: in definition of macro 'memcpy'
#define memcpy(t, f, n) __builtin_memcpy(t, f, n)
^
vim +4118 drivers/gpu/drm/i915/intel_pm.c
4102 mem_attr->mem_wa = WATERMARK_WA_NONE;
4103 return 0;
4104 }
4105
4106 if (!memdev_info->valid)
4107 goto exit;
4108
4109 /*
4110 * We hold wm_mutex lock, so any other flip can't proceed beyond WM
4111 * calculation step until this flip writes modified WM values.
4112 * So it's safe to read plane_state of other pipes without taking CRTC lock
4113 */
4114 ret = drm_modeset_lock(&dev_priv->wm.wm_ww_mutex, state->acquire_ctx);
4115 if (ret)
4116 return ret;
4117
> 4118 memcpy(mem_attr, &dev_priv->wm.skl_hw.mem_attr, sizeof(mem_attr));
4119
4120 for_each_crtc_in_state(state, crtc, cstate, i) {
4121 struct drm_plane *plane;
4122 const struct drm_plane_state *pstate;
4123 int active_planes = 0;
4124 uint32_t max_plane_bw_kbps = 0;
4125
4126 mem_attr->pipe_y_tiled[i] = false;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57990 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/i915/bxt: Enable IPC support
2017-01-31 15:56 ` Ander Conselvan De Oliveira
@ 2017-02-01 6:44 ` Mahesh Kumar
2017-03-15 20:15 ` Paulo Zanoni
0 siblings, 1 reply; 14+ messages in thread
From: Mahesh Kumar @ 2017-02-01 6:44 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, intel-gfx; +Cc: paulo.r.zanoni, maarten.lankhorst
Hi,
On Tuesday 31 January 2017 09:26 PM, Ander Conselvan De Oliveira wrote:
> On Tue, 2017-01-31 at 20:27 +0530, Mahesh Kumar wrote:
>> This patch adds IPC support for platforms. This patch enables IPC
>> only for BXT/KBL platform as for SKL recommendation is to keep it disabled.
>> IPC (Isochronous Priority Control) is the hardware feature, which
>> dynamically controls the memory read priority of Display.
>>
>> When IPC is enabled, plane read requests are sent at high priority until
>> filling above the transition watermark, then the requests are sent at
>> lower priority until dropping below the level 0 watermark.
>> The lower priority requests allow other memory clients to have better
>> memory access. When IPC is disabled, all plane read requests are sent at
>> high priority.
>>
>> Changes since V1:
>> - Remove commandline parameter to disable ipc
>> - Address Paulo's comments
>> Changes since V2:
>> - Address review comments
>> - Set ipc_enabled flag
>> Changes since V3:
>> - move ipc_enabled flag assignment inside intel_ipc_enable function
>> Changes since V4:
>> - Re-enable IPC after suspend/resume
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 4 +++-
>> drivers/gpu/drm/i915/i915_reg.h | 1 +
>> drivers/gpu/drm/i915/intel_display.c | 1 +
>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++++++
>> 5 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index ca168b22ee26..5f3b22946971 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1246,7 +1246,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct
>> pci_device_id *ent)
>>
>> intel_runtime_pm_enable(dev_priv);
>>
>> - dev_priv->ipc_enabled = false;
>> + intel_enable_ipc(dev_priv);
>>
>> /* Everything is in place, we can now relax! */
>> DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
>> @@ -2439,6 +2439,8 @@ static int intel_runtime_resume(struct device *kdev)
>>
>> enable_rpm_wakeref_asserts(dev_priv);
>>
>> + intel_enable_ipc(dev_priv);
>> +
>> if (ret)
>> DRM_ERROR("Runtime resume failed, disabling it (%d)\n", ret);
>> else
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 72f9f36ae5ce..36e0a33f876c 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6466,6 +6466,7 @@ enum {
>> #define DISP_FBC_WM_DIS (1<<15)
>> #define DISP_ARB_CTL2 _MMIO(0x45004)
>> #define DISP_DATA_PARTITION_5_6 (1<<6)
>> +#define DISP_IPC_ENABLE (1<<3)
>> #define DBUF_CTL _MMIO(0x45008)
>> #define DBUF_POWER_REQUEST (1<<31)
>> #define DBUF_POWER_STATE (1<<30)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 0bf8e1bfbe7e..1aa708b6f55e 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -17222,6 +17222,7 @@ void intel_display_resume(struct drm_device *dev)
>> if (!ret)
>> ret = __intel_display_resume(dev, state);
>>
>> + intel_enable_ipc(dev_priv);
>> drm_modeset_drop_locks(&ctx);
>> drm_modeset_acquire_fini(&ctx);
>> mutex_unlock(&dev->mode_config.mutex);
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 0cec0013ace0..ab7423b0a41b 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1787,6 +1787,7 @@ bool skl_ddb_allocation_overlaps(const struct
>> skl_ddb_entry **entries,
>> uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
>> bool ilk_disable_lp_wm(struct drm_device *dev);
>> int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
>> +void intel_enable_ipc(struct drm_i915_private *dev_priv);
>> static inline int intel_enable_rc6(void)
>> {
>> return i915.enable_rc6;
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 249623d45be0..16e83efa1118 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4667,6 +4667,22 @@ void intel_update_watermarks(struct intel_crtc *crtc)
>> dev_priv->display.update_wm(crtc);
>> }
>>
>> +void intel_enable_ipc(struct drm_i915_private *dev_priv)
>> +{
>> + u32 val;
>> +
>> + dev_priv->ipc_enabled = false;
>> + if (!(IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)))
>> + return;
> Do we want this enabled in Geminilake?
>
> Ander
yes, we need to enable this for Gemnilake also, but we need "Transition
WM" patch, before enabling it for GLK.
-Mahesh
>
>> +
>> + val = I915_READ(DISP_ARB_CTL2);
>> +
>> + val |= DISP_IPC_ENABLE;
>> +
>> + I915_WRITE(DISP_ARB_CTL2, val);
>> + dev_priv->ipc_enabled = true;
>> +}
>> +
>> /*
>> * Lock protecting IPS related data structures
>> */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround
2017-01-31 18:58 ` kbuild test robot
@ 2017-02-01 9:24 ` Mahesh Kumar
0 siblings, 0 replies; 14+ messages in thread
From: Mahesh Kumar @ 2017-02-01 9:24 UTC (permalink / raw)
To: intel-gfx
This patch enables workarounds related to display arbitrated memory
bandwidth only if it's necessary. WA's are applicable for all GEN9
based platforms.
Changes since v1:
- Rebase on top of Paulo's patch series
Changes since v2:
- Address review comments
- Rebase/rework as per other patch changes in series
Changes since v3:
- Rebase the patch
- Address Paulo's review comments
- introduce ww_mutex to protect WM operations
- Protect system memory bandwidth calculation with ww_mutex
Changes since v4:
- drop goto label
- fix compilation warning
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 15 ++++
drivers/gpu/drm/i915/intel_display.c | 34 ++++++++
drivers/gpu/drm/i915/intel_pm.c | 161 ++++++++++++++++++++++++++++++-----
4 files changed, 192 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b8d0dd2811a9..a14b2a9d2e6a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -814,6 +814,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
mutex_init(&dev_priv->modeset_restore_lock);
mutex_init(&dev_priv->av_mutex);
mutex_init(&dev_priv->wm.wm_mutex);
+ drm_modeset_lock_init(&dev_priv->wm.wm_ww_mutex);
mutex_init(&dev_priv->pps_mutex);
intel_uc_init_early(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index df24de2b65f2..6e5cdd6c9dfd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1174,6 +1174,13 @@ enum intel_sbi_destination {
SBI_MPHY,
};
+/* SKL+ Watermark arbitrated display bandwidth Workarounds */
+enum watermark_memory_wa {
+ WATERMARK_WA_NONE,
+ WATERMARK_WA_X_TILED,
+ WATERMARK_WA_Y_TILED,
+};
+
#define QUIRK_PIPEA_FORCE (1<<0)
#define QUIRK_LVDS_SSC_DISABLE (1<<1)
#define QUIRK_INVERT_BRIGHTNESS (1<<2)
@@ -1744,8 +1751,15 @@ struct skl_ddb_allocation {
struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
};
+struct skl_mem_bw_attr {
+ enum watermark_memory_wa mem_wa;
+ uint32_t pipe_bw_kbps[I915_MAX_PIPES];
+ bool pipe_y_tiled[I915_MAX_PIPES];
+};
+
struct skl_wm_values {
unsigned dirty_pipes;
+ struct skl_mem_bw_attr mem_attr;
struct skl_ddb_allocation ddb;
};
@@ -2348,6 +2362,7 @@ struct drm_i915_private {
* cstate->wm.need_postvbl_update.
*/
struct mutex wm_mutex;
+ struct drm_modeset_lock wm_ww_mutex; /* protect DDB/WM redistribution among pipes */
/*
* Set during HW readout of watermarks/DDB. Some platforms
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1aa708b6f55e..de512bd8136b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14589,6 +14589,38 @@ static void intel_atomic_track_fbs(struct drm_atomic_state *state)
to_intel_plane(plane)->frontbuffer_bit);
}
+static void
+intel_update_wm_bw_wa(struct drm_atomic_state *state)
+{
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct drm_i915_private *dev_priv = to_i915(state->dev);
+ const struct drm_crtc *crtc;
+ const struct drm_crtc_state *cstate;
+ const struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ struct skl_mem_bw_attr *results = &intel_state->wm_results.mem_attr;
+ struct skl_mem_bw_attr *hw_vals = &dev_priv->wm.skl_hw.mem_attr;
+ int i;
+
+ if (!IS_GEN9(dev_priv))
+ return;
+
+ if (!memdev_info->valid)
+ return;
+
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ hw_vals->pipe_bw_kbps[i] = results->pipe_bw_kbps[i];
+ hw_vals->pipe_y_tiled[i] = results->pipe_y_tiled[i];
+ }
+
+ hw_vals->mem_wa = results->mem_wa;
+
+ /*
+ * As we already updated state variables no need to hold the lock,
+ * other commits can proceed now with their calculation
+ */
+ drm_modeset_unlock(&dev_priv->wm.wm_ww_mutex);
+}
+
/**
* intel_atomic_commit - commit validated state object
* @dev: DRM device
@@ -14629,6 +14661,8 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_shared_dpll_swap_state(state);
intel_atomic_track_fbs(state);
+ intel_update_wm_bw_wa(state);
+
if (intel_state->modeset) {
memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
sizeof(intel_state->min_pixclk));
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 16e83efa1118..aa75558edf1f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2887,20 +2887,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
#define SKL_SAGV_BLOCK_TIME 30 /* µs */
-/*
- * FIXME: We still don't have the proper code detect if we need to apply the WA,
- * so assume we'll always need it in order to avoid underruns.
- */
-static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
-{
- struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-
- if (IS_GEN9_BC(dev_priv) || IS_BROXTON(dev_priv))
- return true;
-
- return false;
-}
-
static bool
intel_has_sagv(struct drm_i915_private *dev_priv)
{
@@ -3049,7 +3034,8 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
latency = dev_priv->wm.skl_latency[level];
- if (skl_needs_memory_bw_wa(intel_state) &&
+ if (intel_state->wm_results.mem_attr.mem_wa !=
+ WATERMARK_WA_NONE &&
plane->base.state->fb->modifier ==
I915_FORMAT_MOD_X_TILED)
latency += 15;
@@ -3581,7 +3567,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
uint32_t y_min_scanlines;
struct intel_atomic_state *state =
to_intel_atomic_state(cstate->base.state);
- bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
+ enum watermark_memory_wa mem_wa;
bool y_tiled, x_tiled;
if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
@@ -3597,7 +3583,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
if (IS_KABYLAKE(dev_priv) && dev_priv->ipc_enabled)
latency += 4;
- if (apply_memory_bw_wa && x_tiled)
+ mem_wa = state->wm_results.mem_attr.mem_wa;
+ if (mem_wa != WATERMARK_WA_NONE && x_tiled)
latency += 15;
width = drm_rect_width(&intel_pstate->base.src) >> 16;
@@ -3632,7 +3619,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
y_min_scanlines = 4;
}
- if (apply_memory_bw_wa)
+ if (mem_wa == WATERMARK_WA_Y_TILED)
y_min_scanlines *= 2;
plane_bytes_per_line = width * cpp;
@@ -4061,6 +4048,16 @@ skl_compute_ddb(struct drm_atomic_state *state)
}
/*
+ * If Watermark workaround is changed we need to recalculate
+ * watermark values for all active pipes
+ */
+ if (intel_state->wm_results.mem_attr.mem_wa !=
+ dev_priv->wm.skl_hw.mem_attr.mem_wa) {
+ realloc_pipes = ~0;
+ intel_state->wm_results.dirty_pipes = ~0;
+ }
+
+ /*
* We're not recomputing for the pipes not included in the commit, so
* make sure we start with the current state.
*/
@@ -4085,6 +4082,128 @@ skl_compute_ddb(struct drm_atomic_state *state)
return 0;
}
+static int
+skl_compute_memory_bandwidth_wm_wa(struct drm_atomic_state *state)
+{
+ struct drm_device *dev = state->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ struct skl_mem_bw_attr *mem_attr = &intel_state->wm_results.mem_attr;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *cstate;
+ int active_pipes = 0;
+ uint32_t max_pipe_bw_kbps = 0, total_pipe_bw_kbps;
+ int display_bw_percentage;
+ bool y_tile_enabled = false;
+ int ret, i;
+
+ if (!IS_GEN9(dev_priv)) {
+ mem_attr->mem_wa = WATERMARK_WA_NONE;
+ return 0;
+ }
+
+ if (!memdev_info->valid) {
+ mem_attr->mem_wa = WATERMARK_WA_Y_TILED;
+ return 0;
+ }
+
+ /*
+ * We hold wm_mutex lock, so any other flip can't proceed beyond WM
+ * calculation step until this flip writes modified WM values.
+ * So it's safe to read plane_state of other pipes without taking CRTC lock
+ */
+ ret = drm_modeset_lock(&dev_priv->wm.wm_ww_mutex, state->acquire_ctx);
+ if (ret)
+ return ret;
+
+ memcpy(mem_attr, &dev_priv->wm.skl_hw.mem_attr, sizeof(*mem_attr));
+
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ struct drm_plane *plane;
+ const struct drm_plane_state *pstate;
+ int active_planes = 0;
+ uint32_t max_plane_bw_kbps = 0;
+
+ mem_attr->pipe_y_tiled[i] = false;
+
+ if (!cstate->active) {
+ mem_attr->pipe_bw_kbps[i] = 0;
+ continue;
+ }
+
+ drm_atomic_crtc_state_for_each_plane_state(plane, pstate,
+ cstate) {
+ struct drm_framebuffer *fb;
+ uint32_t plane_bw_kbps;
+ enum plane_id id = to_intel_plane(plane)->id;
+
+ if (!pstate->visible)
+ continue;
+
+ if (WARN_ON(!pstate->fb))
+ continue;
+
+ if (id == PLANE_CURSOR)
+ continue;
+
+ fb = pstate->fb;
+ if ((fb->modifier == I915_FORMAT_MOD_Y_TILED ||
+ fb->modifier == I915_FORMAT_MOD_Yf_TILED))
+ mem_attr->pipe_y_tiled[i] = true;
+
+ plane_bw_kbps = skl_adjusted_plane_pixel_rate(
+ to_intel_crtc_state(cstate),
+ to_intel_plane_state(pstate));
+ max_plane_bw_kbps = max(plane_bw_kbps,
+ max_plane_bw_kbps);
+ active_planes++;
+ }
+ mem_attr->pipe_bw_kbps[i] = max_plane_bw_kbps * active_planes;
+ }
+
+ for_each_pipe(dev_priv, i) {
+ if (mem_attr->pipe_bw_kbps[i]) {
+ max_pipe_bw_kbps = max(max_pipe_bw_kbps,
+ mem_attr->pipe_bw_kbps[i]);
+ active_pipes++;
+ }
+ if (mem_attr->pipe_y_tiled[i])
+ y_tile_enabled = true;
+ }
+
+ total_pipe_bw_kbps = max_pipe_bw_kbps * active_pipes;
+ display_bw_percentage = DIV_ROUND_UP_ULL((uint64_t)total_pipe_bw_kbps *
+ 100, memdev_info->bandwidth_kbps);
+
+ /*
+ * If there is any Ytile plane enabled and arbitrated display
+ * bandwidth > 20% of raw system memory bandwidth
+ * Enale Y-tile related WA
+ *
+ * If memory is dual channel single rank, Xtile limit = 35%, else Xtile
+ * limit = 60%
+ * If there is no Ytile plane enabled and
+ * arbitrated display bandwidth > Xtile limit
+ * Enable X-tile realated WA
+ */
+ if (y_tile_enabled && (display_bw_percentage > 20))
+ mem_attr->mem_wa = WATERMARK_WA_Y_TILED;
+ else {
+ int x_tile_percentage = 60;
+ enum rank rank = memdev_info->rank;
+
+ if ((rank == I915_DRAM_RANK_SINGLE) &&
+ (memdev_info->num_channels == 2))
+ x_tile_percentage = 35;
+
+ if (display_bw_percentage > x_tile_percentage)
+ mem_attr->mem_wa = WATERMARK_WA_X_TILED;
+ }
+
+ return 0;
+}
+
static void
skl_copy_wm_for_pipe(struct skl_wm_values *dst,
struct skl_wm_values *src,
@@ -4160,6 +4279,10 @@ skl_compute_wm(struct drm_atomic_state *state)
/* Clear all dirty flags */
results->dirty_pipes = 0;
+ ret = skl_compute_memory_bandwidth_wm_wa(state);
+ if (ret)
+ return ret;
+
ret = skl_compute_ddb(state);
if (ret)
return ret;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for Enable IPC & WM related WA's (rev2)
2017-01-31 14:57 [PATCH 0/3] Enable IPC & WM related WA's Mahesh Kumar
` (3 preceding siblings ...)
2017-01-31 15:54 ` ✓ Fi.CI.BAT: success for Enable IPC & WM related WA's Patchwork
@ 2017-02-01 9:54 ` Patchwork
4 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-02-01 9:54 UTC (permalink / raw)
To: Kumar, Mahesh; +Cc: intel-gfx
== Series Details ==
Series: Enable IPC & WM related WA's (rev2)
URL : https://patchwork.freedesktop.org/series/18842/
State : failure
== Summary ==
Series 18842v2 Enable IPC & WM related WA's
https://patchwork.freedesktop.org/api/1.0/series/18842/revisions/2/mbox/
Test core_prop_blob:
Subgroup basic:
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-skl-6260u)
Test drv_hangman:
Subgroup error-state-basic:
pass -> DMESG-WARN (fi-bxt-j4205)
Test drv_module_reload:
Subgroup basic-no-display:
pass -> DMESG-WARN (fi-skl-6700hq)
Subgroup basic-reload:
pass -> DMESG-WARN (fi-skl-6770hq)
Subgroup basic-reload-final:
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6260u)
Test gem_busy:
Subgroup basic-busy-default:
pass -> DMESG-WARN (fi-bxt-j4205)
Subgroup basic-hang-default:
pass -> FAIL (fi-hsw-4770r)
Test gem_close_race:
Subgroup basic-threads:
pass -> DMESG-WARN (fi-bxt-j4205)
Test gem_ctx_basic:
pass -> DMESG-WARN (fi-bxt-j4205)
Test gem_ctx_exec:
Subgroup basic:
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-skl-6260u)
Test gem_ctx_switch:
Subgroup basic-default:
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-skl-6260u)
Test gem_exec_basic:
Subgroup basic-bsd:
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-skl-6260u)
Subgroup basic-render:
pass -> DMESG-WARN (fi-bxt-j4205)
Subgroup gtt-bsd:
pass -> DMESG-WARN (fi-bxt-j4205)
Subgroup gtt-default:
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-kbl-7500u)
Subgroup readonly-blt:
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-skl-6260u)
Subgroup readonly-bsd2:
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-6260u)
Test gem_exec_create:
Subgroup basic:
pass -> DMESG-WARN (fi-bxt-j4205)
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-wb:
pass -> DMESG-WARN (fi-bxt-j4205)
Subgroup basic-uc-pro-default:
pass -> DMESG-WARN (fi-bxt-t5700)
Subgroup basic-uc-ro-default:
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-kbl-7500u)
Subgroup basic-wb-pro-default:
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-skl-6260u)
Subgroup basic-wb-ro-default:
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-kbl-7500u)
WARNING: Long output truncated
83f390eaec64a1b0566f5f747d20741c077fcff5 drm-tip: 2017y-01m-31d-20h-19m-59s UTC integration manifest
cd81dde drm/i915/gen9: WM memory bandwidth related workaround
4d4f76e drm/i915: Decode system memory bandwidth
ff1b382 drm/i915/bxt: Enable IPC support
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3660/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround
2017-01-31 14:57 ` [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround Mahesh Kumar
2017-01-31 17:08 ` kbuild test robot
2017-01-31 18:58 ` kbuild test robot
@ 2017-02-01 9:55 ` Maarten Lankhorst
2 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-02-01 9:55 UTC (permalink / raw)
To: Mahesh Kumar, intel-gfx; +Cc: paulo.r.zanoni, maarten.lankhorst
Hey,
Op 31-01-17 om 15:57 schreef Mahesh Kumar:
> This patch enables workarounds related to display arbitrated memory
> bandwidth only if it's necessary. WA's are applicable for all GEN9
> based platforms.
>
> Changes since v1:
> - Rebase on top of Paulo's patch series
> Changes since v2:
> - Address review comments
> - Rebase/rework as per other patch changes in series
> Changes since v3:
> - Rebase the patch
> - introduce ww_mutex to protect WM operations
> - Protect system memory bandwidth calculation with ww_mutex
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 15 ++++
> drivers/gpu/drm/i915/intel_display.c | 34 ++++++++
> drivers/gpu/drm/i915/intel_pm.c | 163 +++++++++++++++++++++++++++++++----
> 4 files changed, 194 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b8d0dd2811a9..a14b2a9d2e6a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -814,6 +814,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
> mutex_init(&dev_priv->modeset_restore_lock);
> mutex_init(&dev_priv->av_mutex);
> mutex_init(&dev_priv->wm.wm_mutex);
> + drm_modeset_lock_init(&dev_priv->wm.wm_ww_mutex);
> mutex_init(&dev_priv->pps_mutex);
This is not the right approach wrt serialization. In case of a nonblocking update you need to grab the other crtc states so the update is serialized correctly.
Else you will get races with nonblocking modesets.
Please just grab all other crtc mutexes to update. In fact when changing the workaround you probably need to grab the crtc state as well to make sure all previous updates complete. I think in the last iteration I told you what worked, can't remember for sure.
Lastly this won't work as intended, we need to clean up all callers of lock_all_ctx first.
> intel_uc_init_early(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index df24de2b65f2..6e5cdd6c9dfd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1174,6 +1174,13 @@ enum intel_sbi_destination {
> SBI_MPHY,
> };
>
> +/* SKL+ Watermark arbitrated display bandwidth Workarounds */
> +enum watermark_memory_wa {
> + WATERMARK_WA_NONE,
> + WATERMARK_WA_X_TILED,
> + WATERMARK_WA_Y_TILED,
> +};
> +
> #define QUIRK_PIPEA_FORCE (1<<0)
> #define QUIRK_LVDS_SSC_DISABLE (1<<1)
> #define QUIRK_INVERT_BRIGHTNESS (1<<2)
> @@ -1744,8 +1751,15 @@ struct skl_ddb_allocation {
> struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
> };
>
> +struct skl_mem_bw_attr {
> + enum watermark_memory_wa mem_wa;
> + uint32_t pipe_bw_kbps[I915_MAX_PIPES];
> + bool pipe_y_tiled[I915_MAX_PIPES];
> +};
> +
> struct skl_wm_values {
> unsigned dirty_pipes;
> + struct skl_mem_bw_attr mem_attr;
> struct skl_ddb_allocation ddb;
> };
>
> @@ -2348,6 +2362,7 @@ struct drm_i915_private {
> * cstate->wm.need_postvbl_update.
> */
> struct mutex wm_mutex;
> + struct drm_modeset_lock wm_ww_mutex; /* protect DDB/WM redistribution among pipes */
>
> /*
> * Set during HW readout of watermarks/DDB. Some platforms
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1aa708b6f55e..de512bd8136b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14589,6 +14589,38 @@ static void intel_atomic_track_fbs(struct drm_atomic_state *state)
> to_intel_plane(plane)->frontbuffer_bit);
> }
>
> +static void
> +intel_update_wm_bw_wa(struct drm_atomic_state *state)
> +{
> + struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> + struct drm_i915_private *dev_priv = to_i915(state->dev);
> + const struct drm_crtc *crtc;
> + const struct drm_crtc_state *cstate;
> + const struct memdev_info *memdev_info = &dev_priv->memdev_info;
> + struct skl_mem_bw_attr *results = &intel_state->wm_results.mem_attr;
> + struct skl_mem_bw_attr *hw_vals = &dev_priv->wm.skl_hw.mem_attr;
> + int i;
> +
> + if (!IS_GEN9(dev_priv))
> + return;
> +
> + if (!memdev_info->valid)
> + return;
> +
> + for_each_crtc_in_state(state, crtc, cstate, i) {
> + hw_vals->pipe_bw_kbps[i] = results->pipe_bw_kbps[i];
> + hw_vals->pipe_y_tiled[i] = results->pipe_y_tiled[i];
> + }
> +
> + hw_vals->mem_wa = results->mem_wa;
> +
> + /*
> + * As we already updated state variables no need to hold the lock,
> + * other commits can proceed now with their calculation
> + */
> + drm_modeset_unlock(&dev_priv->wm.wm_ww_mutex);
> +}
Ugh, please don't do this. Ever. Locking is the caller's responsibility
> /**
> * intel_atomic_commit - commit validated state object
> * @dev: DRM device
> @@ -14629,6 +14661,8 @@ static int intel_atomic_commit(struct drm_device *dev,
> intel_shared_dpll_swap_state(state);
> intel_atomic_track_fbs(state);
>
> + intel_update_wm_bw_wa(state);
> +
> if (intel_state->modeset) {
> memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
> sizeof(intel_state->min_pixclk));
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 16e83efa1118..ae43df5cdfd8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2887,21 +2887,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
>
> #define SKL_SAGV_BLOCK_TIME 30 /* µs */
>
> -/*
> - * FIXME: We still don't have the proper code detect if we need to apply the WA,
> - * so assume we'll always need it in order to avoid underruns.
> - */
> -static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> -{
> - struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -
> - if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> - IS_KABYLAKE(dev_priv))
> - return true;
> -
> - return false;
> -}
> -
> static bool
> intel_has_sagv(struct drm_i915_private *dev_priv)
> {
> @@ -3049,7 +3034,8 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>
> latency = dev_priv->wm.skl_latency[level];
>
> - if (skl_needs_memory_bw_wa(intel_state) &&
> + if (intel_state->wm_results.mem_attr.mem_wa !=
> + WATERMARK_WA_NONE &&
> plane->base.state->fb->modifier ==
> I915_FORMAT_MOD_X_TILED)
> latency += 15;
> @@ -3581,7 +3567,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> uint32_t y_min_scanlines;
> struct intel_atomic_state *state =
> to_intel_atomic_state(cstate->base.state);
> - bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
> + enum watermark_memory_wa mem_wa;
> bool y_tiled, x_tiled;
>
> if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
> @@ -3597,7 +3583,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> if (IS_KABYLAKE(dev_priv) && dev_priv->ipc_enabled)
> latency += 4;
>
> - if (apply_memory_bw_wa && x_tiled)
> + mem_wa = state->wm_results.mem_attr.mem_wa;
> + if (mem_wa != WATERMARK_WA_NONE && x_tiled)
> latency += 15;
>
> width = drm_rect_width(&intel_pstate->base.src) >> 16;
> @@ -3632,7 +3619,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> y_min_scanlines = 4;
> }
>
> - if (apply_memory_bw_wa)
> + if (mem_wa == WATERMARK_WA_Y_TILED)
> y_min_scanlines *= 2;
>
> plane_bytes_per_line = width * cpp;
> @@ -4061,6 +4048,16 @@ skl_compute_ddb(struct drm_atomic_state *state)
> }
>
> /*
> + * If Watermark workaround is changed we need to recalculate
> + * watermark values for all active pipes
> + */
> + if (intel_state->wm_results.mem_attr.mem_wa !=
> + dev_priv->wm.skl_hw.mem_attr.mem_wa) {
> + realloc_pipes = ~0;
> + intel_state->wm_results.dirty_pipes = ~0;
> + }
> +
> + /*
> * We're not recomputing for the pipes not included in the commit, so
> * make sure we start with the current state.
> */
> @@ -4085,6 +4082,130 @@ skl_compute_ddb(struct drm_atomic_state *state)
> return 0;
> }
>
> +static int
> +skl_compute_memory_bandwidth_wm_wa(struct drm_atomic_state *state)
> +{
> + struct drm_device *dev = state->dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> + struct memdev_info *memdev_info = &dev_priv->memdev_info;
> + struct skl_mem_bw_attr *mem_attr = &intel_state->wm_results.mem_attr;
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *cstate;
> + int active_pipes = 0;
> + uint32_t max_pipe_bw_kbps = 0, total_pipe_bw_kbps;
> + int display_bw_percentage;
> + bool y_tile_enabled = false;
> + int ret, i;
> +
> + if (!IS_GEN9(dev_priv)) {
> + mem_attr->mem_wa = WATERMARK_WA_NONE;
> + return 0;
> + }
> +
> + if (!memdev_info->valid)
> + goto exit;
> +
> + /*
> + * We hold wm_mutex lock, so any other flip can't proceed beyond WM
> + * calculation step until this flip writes modified WM values.
> + * So it's safe to read plane_state of other pipes without taking CRTC lock
> + */
> + ret = drm_modeset_lock(&dev_priv->wm.wm_ww_mutex, state->acquire_ctx);
> + if (ret)
> + return ret;
> +
> + memcpy(mem_attr, &dev_priv->wm.skl_hw.mem_attr, sizeof(mem_attr));
> +
> + for_each_crtc_in_state(state, crtc, cstate, i) {
> + struct drm_plane *plane;
> + const struct drm_plane_state *pstate;
> + int active_planes = 0;
> + uint32_t max_plane_bw_kbps = 0;
> +
> + mem_attr->pipe_y_tiled[i] = false;
> +
> + if (!cstate->active) {
> + mem_attr->pipe_bw_kbps[i] = 0;
> + continue;
> + }
> +
> + drm_atomic_crtc_state_for_each_plane_state(plane, pstate,
> + cstate) {
> + struct drm_framebuffer *fb;
> + uint32_t plane_bw_kbps;
> + enum plane_id id = to_intel_plane(plane)->id;
> +
> + if (!pstate->visible)
> + continue;
> +
> + if (WARN_ON(!pstate->fb))
> + continue;
> +
> + if (id == PLANE_CURSOR)
> + continue;
> +
> + fb = pstate->fb;
> + if ((fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> + fb->modifier == I915_FORMAT_MOD_Yf_TILED))
> + mem_attr->pipe_y_tiled[i] = true;
> +
> + plane_bw_kbps = skl_adjusted_plane_pixel_rate(
> + to_intel_crtc_state(cstate),
> + to_intel_plane_state(pstate));
> + max_plane_bw_kbps = max(plane_bw_kbps,
> + max_plane_bw_kbps);
> + active_planes++;
> + }
> + mem_attr->pipe_bw_kbps[i] = max_plane_bw_kbps * active_planes;
> + }
> +
> + for_each_pipe(dev_priv, i) {
> + if (mem_attr->pipe_bw_kbps[i]) {
> + max_pipe_bw_kbps = max(max_pipe_bw_kbps,
> + mem_attr->pipe_bw_kbps[i]);
> + active_pipes++;
> + }
> + if (mem_attr->pipe_y_tiled[i])
> + y_tile_enabled = true;
> + }
> +
> + total_pipe_bw_kbps = max_pipe_bw_kbps * active_pipes;
> + display_bw_percentage = DIV_ROUND_UP_ULL((uint64_t)total_pipe_bw_kbps *
> + 100, memdev_info->bandwidth_kbps);
> +
> + /*
> + * If there is any Ytile plane enabled and arbitrated display
> + * bandwidth > 20% of raw system memory bandwidth
> + * Enale Y-tile related WA
> + *
> + * If memory is dual channel single rank, Xtile limit = 35%, else Xtile
> + * limit = 60%
> + * If there is no Ytile plane enabled and
> + * arbitrated display bandwidth > Xtile limit
> + * Enable X-tile realated WA
> + */
> + if (y_tile_enabled && (display_bw_percentage > 20))
> + mem_attr->mem_wa = WATERMARK_WA_Y_TILED;
> + else {
> + int x_tile_percentage = 60;
> + enum rank rank = memdev_info->rank;
> +
> + if ((rank == I915_DRAM_RANK_SINGLE) &&
> + (memdev_info->num_channels == 2))
> + x_tile_percentage = 35;
> +
> + if (display_bw_percentage > x_tile_percentage)
> + mem_attr->mem_wa = WATERMARK_WA_X_TILED;
> + }
> +
> + return 0;
> +
> +exit:
> + mem_attr->mem_wa = WATERMARK_WA_Y_TILED;
> + return 0;
> +}
> +
> static void
> skl_copy_wm_for_pipe(struct skl_wm_values *dst,
> struct skl_wm_values *src,
> @@ -4160,6 +4281,10 @@ skl_compute_wm(struct drm_atomic_state *state)
> /* Clear all dirty flags */
> results->dirty_pipes = 0;
>
> + ret = skl_compute_memory_bandwidth_wm_wa(state);
> + if (ret)
> + return ret;
> +
> ret = skl_compute_ddb(state);
> if (ret)
> return ret;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/i915/bxt: Enable IPC support
2017-02-01 6:44 ` Mahesh Kumar
@ 2017-03-15 20:15 ` Paulo Zanoni
0 siblings, 0 replies; 14+ messages in thread
From: Paulo Zanoni @ 2017-03-15 20:15 UTC (permalink / raw)
To: Mahesh Kumar, Ander Conselvan De Oliveira, intel-gfx; +Cc: maarten.lankhorst
Em Qua, 2017-02-01 às 12:14 +0530, Mahesh Kumar escreveu:
> Hi,
>
>
> On Tuesday 31 January 2017 09:26 PM, Ander Conselvan De Oliveira
> wrote:
> >
> > On Tue, 2017-01-31 at 20:27 +0530, Mahesh Kumar wrote:
> > >
> > > This patch adds IPC support for platforms. This patch enables IPC
> > > only for BXT/KBL platform as for SKL recommendation is to keep it
> > > disabled.
> > > IPC (Isochronous Priority Control) is the hardware feature, which
> > > dynamically controls the memory read priority of Display.
> > >
> > > When IPC is enabled, plane read requests are sent at high
> > > priority until
> > > filling above the transition watermark, then the requests are
> > > sent at
> > > lower priority until dropping below the level 0 watermark.
> > > The lower priority requests allow other memory clients to have
> > > better
> > > memory access. When IPC is disabled, all plane read requests are
> > > sent at
> > > high priority.
> > >
> > > Changes since V1:
> > > - Remove commandline parameter to disable ipc
> > > - Address Paulo's comments
> > > Changes since V2:
> > > - Address review comments
> > > - Set ipc_enabled flag
> > > Changes since V3:
> > > - move ipc_enabled flag assignment inside intel_ipc_enable
> > > function
> > > Changes since V4:
> > > - Re-enable IPC after suspend/resume
> > >
> > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 4 +++-
> > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > drivers/gpu/drm/i915/intel_display.c | 1 +
> > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++++++
> > > 5 files changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index ca168b22ee26..5f3b22946971 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -1246,7 +1246,7 @@ int i915_driver_load(struct pci_dev *pdev,
> > > const struct
> > > pci_device_id *ent)
> > >
> > > intel_runtime_pm_enable(dev_priv);
> > >
> > > - dev_priv->ipc_enabled = false;
> > > + intel_enable_ipc(dev_priv);
> > >
> > > /* Everything is in place, we can now relax! */
> > > DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor
> > > %d\n",
> > > @@ -2439,6 +2439,8 @@ static int intel_runtime_resume(struct
> > > device *kdev)
> > >
> > > enable_rpm_wakeref_asserts(dev_priv);
> > >
> > > + intel_enable_ipc(dev_priv);
> > > +
> > > if (ret)
> > > DRM_ERROR("Runtime resume failed, disabling it
> > > (%d)\n", ret);
> > > else
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 72f9f36ae5ce..36e0a33f876c 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6466,6 +6466,7 @@ enum {
> > > #define DISP_FBC_WM_DIS (1<<15)
> > > #define DISP_ARB_CTL2 _MMIO(0x45004)
> > > #define DISP_DATA_PARTITION_5_6 (1<<6)
> > > +#define DISP_IPC_ENABLE (1<<3)
> > > #define DBUF_CTL _MMIO(0x45008)
> > > #define DBUF_POWER_REQUEST (1<<31)
> > > #define DBUF_POWER_STATE (1<<30)
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 0bf8e1bfbe7e..1aa708b6f55e 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -17222,6 +17222,7 @@ void intel_display_resume(struct
> > > drm_device *dev)
> > > if (!ret)
> > > ret = __intel_display_resume(dev, state);
> > >
> > > + intel_enable_ipc(dev_priv);
> > > drm_modeset_drop_locks(&ctx);
> > > drm_modeset_acquire_fini(&ctx);
> > > mutex_unlock(&dev->mode_config.mutex);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 0cec0013ace0..ab7423b0a41b 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1787,6 +1787,7 @@ bool skl_ddb_allocation_overlaps(const
> > > struct
> > > skl_ddb_entry **entries,
> > > uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state
> > > *pipe_config);
> > > bool ilk_disable_lp_wm(struct drm_device *dev);
> > > int sanitize_rc6_option(struct drm_i915_private *dev_priv, int
> > > enable_rc6);
> > > +void intel_enable_ipc(struct drm_i915_private *dev_priv);
> > > static inline int intel_enable_rc6(void)
> > > {
> > > return i915.enable_rc6;
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 249623d45be0..16e83efa1118 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -4667,6 +4667,22 @@ void intel_update_watermarks(struct
> > > intel_crtc *crtc)
> > > dev_priv->display.update_wm(crtc);
> > > }
> > >
> > > +void intel_enable_ipc(struct drm_i915_private *dev_priv)
> > > +{
> > > + u32 val;
> > > +
> > > + dev_priv->ipc_enabled = false;
> > > + if (!(IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)))
> > > + return;
> > Do we want this enabled in Geminilake?
> >
> > Ander
> yes, we need to enable this for Gemnilake also, but we need
> "Transition
> WM" patch, before enabling it for GLK.
Why do we need transition WMs before enabling IPC on GLK but not on the
other platforms?
>
> -Mahesh
> >
> >
> > >
> > > +
> > > + val = I915_READ(DISP_ARB_CTL2);
> > > +
> > > + val |= DISP_IPC_ENABLE;
> > > +
> > > + I915_WRITE(DISP_ARB_CTL2, val);
> > > + dev_priv->ipc_enabled = true;
> > > +}
> > > +
> > > /*
> > > * Lock protecting IPS related data structures
> > > */
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround
2017-02-15 14:48 [PATCH 0/3] Enable IPC & WM related WA's Mahesh Kumar
@ 2017-02-15 14:48 ` Mahesh Kumar
0 siblings, 0 replies; 14+ messages in thread
From: Mahesh Kumar @ 2017-02-15 14:48 UTC (permalink / raw)
To: intel-gfx; +Cc: paulo.r.zanoni, maarten.lankhorst
This patch disables workarounds related to display arbitrated memory
bandwidth if it's not required. WA's are applicable for all GEN9
based platforms.
Changes since v1:
- Rebase on top of Paulo's patch series
Changes since v2:
- Address review comments
- Rebase/rework as per other patch changes in series
Changes since v3:
- Rebase the patch
- Address Paulo's review comments
- introduce ww_mutex to protect WM operations
- Protect system memory bandwidth calculation with ww_mutex
Changes since v4:
- drop goto label
- fix compilation warnings
- Add crtc only if it's WM got changed
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 15 +++
drivers/gpu/drm/i915/intel_display.c | 34 +++++++
drivers/gpu/drm/i915/intel_pm.c | 187 ++++++++++++++++++++++++++++++-----
4 files changed, 213 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9203dad1db07..126c11060ea7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -835,6 +835,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
mutex_init(&dev_priv->modeset_restore_lock);
mutex_init(&dev_priv->av_mutex);
mutex_init(&dev_priv->wm.wm_mutex);
+ drm_modeset_lock_init(&dev_priv->wm.wm_ww_mutex);
mutex_init(&dev_priv->pps_mutex);
intel_uc_init_early(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a88b2721dbe7..974823838da9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1189,6 +1189,13 @@ enum intel_sbi_destination {
SBI_MPHY,
};
+/* SKL+ Watermark arbitrated display bandwidth Workarounds */
+enum watermark_memory_wa {
+ WATERMARK_WA_NONE,
+ WATERMARK_WA_X_TILED,
+ WATERMARK_WA_Y_TILED,
+};
+
#define QUIRK_PIPEA_FORCE (1<<0)
#define QUIRK_LVDS_SSC_DISABLE (1<<1)
#define QUIRK_INVERT_BRIGHTNESS (1<<2)
@@ -1754,8 +1761,15 @@ struct skl_ddb_allocation {
struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
};
+struct skl_mem_bw_attr {
+ enum watermark_memory_wa mem_wa;
+ uint32_t pipe_bw_kbps[I915_MAX_PIPES];
+ bool pipe_y_tiled[I915_MAX_PIPES];
+};
+
struct skl_wm_values {
unsigned dirty_pipes;
+ struct skl_mem_bw_attr mem_attr;
struct skl_ddb_allocation ddb;
};
@@ -2375,6 +2389,7 @@ struct drm_i915_private {
* cstate->wm.need_postvbl_update.
*/
struct mutex wm_mutex;
+ struct drm_modeset_lock wm_ww_mutex; /* protect DDB/WM redistribution among pipes */
/*
* Set during HW readout of watermarks/DDB. Some platforms
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0e92f9bdda1f..76f78879f6e3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13030,6 +13030,38 @@ static void intel_atomic_track_fbs(struct drm_atomic_state *state)
to_intel_plane(plane)->frontbuffer_bit);
}
+static void
+intel_update_wm_bw_wa(struct drm_atomic_state *state)
+{
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct drm_i915_private *dev_priv = to_i915(state->dev);
+ const struct drm_crtc *crtc;
+ const struct drm_crtc_state *cstate;
+ const struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ struct skl_mem_bw_attr *results = &intel_state->wm_results.mem_attr;
+ struct skl_mem_bw_attr *hw_vals = &dev_priv->wm.skl_hw.mem_attr;
+ int i;
+
+ if (!IS_GEN9(dev_priv))
+ return;
+
+ if (!memdev_info->valid)
+ return;
+
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ hw_vals->pipe_bw_kbps[i] = results->pipe_bw_kbps[i];
+ hw_vals->pipe_y_tiled[i] = results->pipe_y_tiled[i];
+ }
+
+ hw_vals->mem_wa = results->mem_wa;
+
+ /*
+ * As we already updated state variables no need to hold the lock,
+ * other commits can proceed now with their calculation
+ */
+ drm_modeset_unlock(&dev_priv->wm.wm_ww_mutex);
+}
+
/**
* intel_atomic_commit - commit validated state object
* @dev: DRM device
@@ -13070,6 +13102,8 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_shared_dpll_swap_state(state);
intel_atomic_track_fbs(state);
+ intel_update_wm_bw_wa(state);
+
if (intel_state->modeset) {
memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
sizeof(intel_state->min_pixclk));
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e578d7f9f871..7c0113bf2387 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2867,20 +2867,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
#define SKL_SAGV_BLOCK_TIME 30 /* µs */
-/*
- * FIXME: We still don't have the proper code detect if we need to apply the WA,
- * so assume we'll always need it in order to avoid underruns.
- */
-static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
-{
- struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-
- if (IS_GEN9_BC(dev_priv) || IS_BROXTON(dev_priv))
- return true;
-
- return false;
-}
-
static bool
intel_has_sagv(struct drm_i915_private *dev_priv)
{
@@ -3028,7 +3014,8 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
latency = dev_priv->wm.skl_latency[level];
- if (skl_needs_memory_bw_wa(intel_state) &&
+ if (intel_state->wm_results.mem_attr.mem_wa !=
+ WATERMARK_WA_NONE &&
plane->base.state->fb->modifier ==
I915_FORMAT_MOD_X_TILED)
latency += 15;
@@ -3560,7 +3547,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
uint32_t y_min_scanlines;
struct intel_atomic_state *state =
to_intel_atomic_state(cstate->base.state);
- bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
+ enum watermark_memory_wa mem_wa = state->wm_results.mem_attr.mem_wa;
bool y_tiled, x_tiled;
if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
@@ -3576,7 +3563,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
if (IS_KABYLAKE(dev_priv) && dev_priv->ipc_enabled)
latency += 4;
- if (apply_memory_bw_wa && x_tiled)
+ if (mem_wa != WATERMARK_WA_NONE && x_tiled)
latency += 15;
width = drm_rect_width(&intel_pstate->base.src) >> 16;
@@ -3611,7 +3598,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
y_min_scanlines = 4;
}
- if (apply_memory_bw_wa)
+ if (mem_wa == WATERMARK_WA_Y_TILED)
y_min_scanlines *= 2;
plane_bytes_per_line = width * cpp;
@@ -4064,6 +4051,128 @@ skl_compute_ddb(struct drm_atomic_state *state)
return 0;
}
+static int
+skl_compute_memory_bandwidth_wm_wa(struct drm_atomic_state *state)
+{
+ struct drm_device *dev = state->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct memdev_info *memdev_info = &dev_priv->memdev_info;
+ struct skl_mem_bw_attr *mem_attr = &intel_state->wm_results.mem_attr;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *cstate;
+ int active_pipes = 0;
+ uint32_t max_pipe_bw_kbps = 0, total_pipe_bw_kbps;
+ int display_bw_percentage;
+ bool y_tile_enabled = false;
+ int ret, i;
+
+ if (!IS_GEN9(dev_priv)) {
+ mem_attr->mem_wa = WATERMARK_WA_NONE;
+ return 0;
+ }
+
+ if (!memdev_info->valid) {
+ mem_attr->mem_wa = WATERMARK_WA_Y_TILED;
+ return 0;
+ }
+
+ /*
+ * We hold wm_mutex lock, so any other flip can't proceed beyond WM
+ * calculation step until this flip writes modified WM values.
+ * So it's safe to read plane_state of other pipes without taking CRTC lock
+ */
+ ret = drm_modeset_lock(&dev_priv->wm.wm_ww_mutex, state->acquire_ctx);
+ if (ret)
+ return ret;
+
+ memcpy(mem_attr, &dev_priv->wm.skl_hw.mem_attr, sizeof(*mem_attr));
+
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ struct drm_plane *plane;
+ const struct drm_plane_state *pstate;
+ int active_planes = 0;
+ uint32_t max_plane_bw_kbps = 0;
+
+ mem_attr->pipe_y_tiled[i] = false;
+
+ if (!cstate->active) {
+ mem_attr->pipe_bw_kbps[i] = 0;
+ continue;
+ }
+
+ drm_atomic_crtc_state_for_each_plane_state(plane, pstate,
+ cstate) {
+ struct drm_framebuffer *fb;
+ uint32_t plane_bw_kbps;
+ enum plane_id id = to_intel_plane(plane)->id;
+
+ if (!pstate->visible)
+ continue;
+
+ if (WARN_ON(!pstate->fb))
+ continue;
+
+ if (id == PLANE_CURSOR)
+ continue;
+
+ fb = pstate->fb;
+ if ((fb->modifier == I915_FORMAT_MOD_Y_TILED ||
+ fb->modifier == I915_FORMAT_MOD_Yf_TILED))
+ mem_attr->pipe_y_tiled[i] = true;
+
+ plane_bw_kbps = skl_adjusted_plane_pixel_rate(
+ to_intel_crtc_state(cstate),
+ to_intel_plane_state(pstate));
+ max_plane_bw_kbps = max(plane_bw_kbps,
+ max_plane_bw_kbps);
+ active_planes++;
+ }
+ mem_attr->pipe_bw_kbps[i] = max_plane_bw_kbps * active_planes;
+ }
+
+ for_each_pipe(dev_priv, i) {
+ if (mem_attr->pipe_bw_kbps[i]) {
+ max_pipe_bw_kbps = max(max_pipe_bw_kbps,
+ mem_attr->pipe_bw_kbps[i]);
+ active_pipes++;
+ }
+ if (mem_attr->pipe_y_tiled[i])
+ y_tile_enabled = true;
+ }
+
+ total_pipe_bw_kbps = max_pipe_bw_kbps * active_pipes;
+ display_bw_percentage = DIV_ROUND_UP_ULL((uint64_t)total_pipe_bw_kbps *
+ 100, memdev_info->bandwidth_kbps);
+
+ /*
+ * If there is any Ytile plane enabled and arbitrated display
+ * bandwidth > 20% of raw system memory bandwidth
+ * Enale Y-tile related WA
+ *
+ * If memory is dual channel single rank, Xtile limit = 35%, else Xtile
+ * limit = 60%
+ * If there is no Ytile plane enabled and
+ * arbitrated display bandwidth > Xtile limit
+ * Enable X-tile realated WA
+ */
+ if (y_tile_enabled && (display_bw_percentage > 20))
+ mem_attr->mem_wa = WATERMARK_WA_Y_TILED;
+ else {
+ int x_tile_percentage = 60;
+ enum memdev_rank rank = memdev_info->rank;
+
+ if ((rank == I915_DRAM_RANK_SINGLE) &&
+ (memdev_info->num_channels == 2))
+ x_tile_percentage = 35;
+
+ if (display_bw_percentage > x_tile_percentage)
+ mem_attr->mem_wa = WATERMARK_WA_X_TILED;
+ }
+
+ return 0;
+}
+
static void
skl_copy_wm_for_pipe(struct skl_wm_values *dst,
struct skl_wm_values *src,
@@ -4118,7 +4227,10 @@ skl_compute_wm(struct drm_atomic_state *state)
struct drm_crtc *crtc;
struct drm_crtc_state *cstate;
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct drm_device *dev = state->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
struct skl_wm_values *results = &intel_state->wm_results;
+ struct intel_crtc *intel_crtc;
struct skl_pipe_wm *pipe_wm;
bool changed = false;
int ret, i;
@@ -4139,6 +4251,10 @@ skl_compute_wm(struct drm_atomic_state *state)
/* Clear all dirty flags */
results->dirty_pipes = 0;
+ ret = skl_compute_memory_bandwidth_wm_wa(state);
+ if (ret)
+ return ret;
+
ret = skl_compute_ddb(state);
if (ret)
return ret;
@@ -4153,20 +4269,43 @@ skl_compute_wm(struct drm_atomic_state *state)
* should allow skl_update_pipe_wm() to return failure in cases where
* no suitable watermark values can be found.
*/
- for_each_crtc_in_state(state, crtc, cstate, i) {
- struct intel_crtc_state *intel_cstate =
- to_intel_crtc_state(cstate);
- const struct skl_pipe_wm *old_pipe_wm =
- &to_intel_crtc_state(crtc->state)->wm.skl.optimal;
+ for_each_intel_crtc(dev, intel_crtc) {
+ struct intel_crtc_state *intel_cstate;
+ const struct skl_pipe_wm *old_pipe_wm;
+ /*
+ * We already hold a wm ww_mutex here, so we can peek into
+ * other pipe's crtc_state without any race condition.
+ */
+ crtc = &intel_crtc->base;
+ cstate = drm_atomic_get_existing_crtc_state(state, crtc);
+ if (!cstate) {
+ if (intel_state->wm_results.mem_attr.mem_wa !=
+ dev_priv->wm.skl_hw.mem_attr.mem_wa)
+ cstate = crtc->state;
+ else
+ continue;
+ }
+
+ intel_cstate = to_intel_crtc_state(cstate);
+ old_pipe_wm = &to_intel_crtc_state(crtc->state)->wm.skl.optimal;
pipe_wm = &intel_cstate->wm.skl.optimal;
ret = skl_update_pipe_wm(cstate, old_pipe_wm, pipe_wm,
&results->ddb, &changed);
if (ret)
return ret;
- if (changed)
+ if (changed) {
+ /*
+ * If WM is changed for crtc and crtc is not already
+ * part of drm_atomic_state, add crtc to
+ * drm_atomic_state.
+ */
+ cstate = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(cstate))
+ return PTR_ERR(cstate);
results->dirty_pipes |= drm_crtc_mask(crtc);
+ }
if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0)
/* This pipe's WM's did not change */
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-03-15 20:15 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-31 14:57 [PATCH 0/3] Enable IPC & WM related WA's Mahesh Kumar
2017-01-31 14:57 ` [PATCH 1/3] drm/i915/bxt: Enable IPC support Mahesh Kumar
2017-01-31 15:56 ` Ander Conselvan De Oliveira
2017-02-01 6:44 ` Mahesh Kumar
2017-03-15 20:15 ` Paulo Zanoni
2017-01-31 14:57 ` [PATCH 2/3] drm/i915: Decode system memory bandwidth Mahesh Kumar
2017-01-31 14:57 ` [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround Mahesh Kumar
2017-01-31 17:08 ` kbuild test robot
2017-01-31 18:58 ` kbuild test robot
2017-02-01 9:24 ` Mahesh Kumar
2017-02-01 9:55 ` Maarten Lankhorst
2017-01-31 15:54 ` ✓ Fi.CI.BAT: success for Enable IPC & WM related WA's Patchwork
2017-02-01 9:54 ` ✗ Fi.CI.BAT: failure for Enable IPC & WM related WA's (rev2) Patchwork
2017-02-15 14:48 [PATCH 0/3] Enable IPC & WM related WA's Mahesh Kumar
2017-02-15 14:48 ` [PATCH 3/3] drm/i915/gen9: WM memory bandwidth related workaround Mahesh Kumar
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.