<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> Hi, <div class="moz-cite-prefix">On Friday 29 September 2017 12:21 AM, Rodrigo Vivi wrote: </div> <blockquote type="cite" cite="mid:20170928185148.4829-2-rodrigo.vivi@intel.com"> <pre wrap="">According to Spec for SKL+: "Isochronous Priority Control. If enabled, Display sends demoted requests once the transition watermark is reached. If transition watermark is not enabled, Display sends demoted requests when the display buffer is full." The commit 'e57f1c02155f ("drm/i915/gen9+: Add has_ipc flag in device info structure")' introduced that as gen9+ but missing many SKL Skus. I believe the reason for that is Spec also mentions workarounds for SKL-ALL: "IPC (Isoch Priority Control) may cause underflows WA: Do not enable IPC in register ARB_CTL2"</pre> </blockquote> yes, it was intentional as you correctly pointed out WA #0477. <blockquote type="cite" cite="mid:20170928185148.4829-2-rodrigo.vivi@intel.com"> <pre wrap=""> It seems lame to add the feature and forever disable it, but it will avoid a mistake of enabling it when we are reorganizing the feature definitions on i915_pci.c later.</pre> </blockquote> sounds Good, <blockquote type="cite" cite="mid:20170928185148.4829-2-rodrigo.vivi@intel.com"> <pre wrap=""> It will also allow us to probably extend that workaround for other platforms. Cc: Mahesh Kumar <a class="moz-txt-link-rfc2396E" href="mailto:mahesh1.kumar@intel.com"><mahesh1.kumar@intel.com></a> Cc: Maarten Lankhorst <a class="moz-txt-link-rfc2396E" href="mailto:maarten.lankhorst@linux.intel.com"><maarten.lankhorst@linux.intel.com></a> Cc: Chris Wilson <a class="moz-txt-link-rfc2396E" href="mailto:chris@chris-wilson.co.uk"><chris@chris-wilson.co.uk></a> Signed-off-by: Rodrigo Vivi <a class="moz-txt-link-rfc2396E" href="mailto:rodrigo.vivi@intel.com"><rodrigo.vivi@intel.com></a> --- drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/intel_pm.c | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index da60866b6628..df751a152057 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -426,6 +426,7 @@ static const struct intel_device_info intel_cherryview_info __initconst = { .platform = INTEL_SKYLAKE, \ .has_csr = 1, \ .has_guc = 1, \ + .has_ipc = 1, \ .ddb_size = 896 static const struct intel_device_info intel_skylake_gt1_info __initconst = { diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 52c4c194aa51..ede871b7982e 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5828,6 +5828,12 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv) { u32 val; + /* Display WA #0477 WaDisableIPC: skl */ + if (IS_SKYLAKE(dev_priv)) { + dev_priv->ipc_enabled = false; + return; + } +</pre> </blockquote> looks good to me. Reviewed-by: Mahesh Kumar <a class="moz-txt-link-rfc2396E" href="mailto:mahesh1.kumar@intel.com"><mahesh1.kumar@intel.com></a> -Mahesh <blockquote type="cite" cite="mid:20170928185148.4829-2-rodrigo.vivi@intel.com"> <pre wrap=""> val = I915_READ(DISP_ARB_CTL2); if (dev_priv->ipc_enabled) </pre> </blockquote> </body> </html>