Hi,
On Tuesday 22 November 2016 08:15 PM, Paulo Zanoni wrote:
Em Ter, 2016-11-22 às 19:05 +0530, Mahesh Kumar escreveu:
Hi,
On Tuesday 22 November 2016 12:16 AM, Paulo Zanoni wrote:
Em Sex, 2016-11-18 às 20:39 +0530, Mahesh Kumar escreveu:
This patch adds IPC support for platforms. This patch enables IPC
only for BXT/KBL platform as for SKL recommendation is to keep is
disabled.
IPC (Isochronous Priority Control) is the hardware feature, which
dynamically controles 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
Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 1 +
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++++
4 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 1b0a589..4074601 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1244,6 +1244,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);
So now we have to places that touch dev_priv->ipc_enabled. This one
and
intel_enable_ipc(). Please move that "dev_priv->ipc_enabled =
false"
line to inside intel_enable_ipc(). It's much easier to read the
code
when there's a single function responsible for setting the
appropriate
value to a variable.
Besides, my understanding of your discussion with Maarten in the
last
revision of this patch was that we needed to change where
intel_enable_ipc() is called in order to make sure the bit stays
enabled after suspend/resume. If that's not needed, why is it not
needed?
We don't overwrite DISP_ARB_CTL2 register during suspend/resume
So there will not be any impact of it & handling during
suspend/resume
is not needed.
Just to make sure I understood: any value that our driver sets to this
register will persist after a suspend/resume cycle? That's not what I
was expecting. Usually after a suspend/resume cycle a lot of registers
are reset to their default value, so our driver needs to write them all
again.
Can you also confirm whether this is true during runtime PM?
Thanks,
Paulo
After discussing with HW team, My understanding was not fully correct.
HW/(DMC fw) does the restoring of registers only in case of DC5/6,
In case of DC9 sw need to take-care of restoring. Will add restore part
in intel_display_resume & intel_runtime_resume.
above two places are correct places for restoring?
Regards,
-Mahesh
thanks for review
Regards,
-Mahesh
/* Everything is in place, we can now relax! */
DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor
%d\n",
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index c70c07a..300418a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6076,6 +6076,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_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index cd132c2..ad542a2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1745,6 +1745,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 df39b50..d8090aa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4682,6 +4682,21 @@ 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;
+
+ 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
*/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
