| Message ID | 20200127205057.831198-1-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v3] drm/i915/tgl: Suppress DC5/DC6 around DSB usage | expand |
On Mon, Jan 27, 2020 at 12:50:57PM -0800, Matt Roper wrote: > There are reports of unexpected DSB busy/timeout happening after IGT > tests finish running that apparently go away when the DMC firmware isn't > loaded. The bspec doesn't say anything specific about DSB needing us to > exit DC5/DC6, but let's try adding DSB usage to the "DC off" list and > see if that changes the behavior. > > v2: Include intel_wakeref.h from intel_dsb.h to ensure the header stays > self-contained. (CI) > > v3: Move intel_display_power_get() call earlier to cover cases where > dsb->refcount is already non-zero and we return early. Also don't > drop the wakeref at the end of the 'get' routine; wait until the > 'put' for that, even in error conditions. (Swati) > > Cc: Swati Sharma <swati2.sharma@intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > Reviewed-by(v1): José Roberto de Souza <jose.souza@intel.com> Sounds like this did not fix DSB failures being seen, so DC states don't appear to be the culprit; we can disregard this patch. More investigation will be necessary to track down the true root cause. Matt > --- > drivers/gpu/drm/i915/display/intel_display_power.c | 3 +++ > drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > drivers/gpu/drm/i915/display/intel_dsb.c | 10 ++++------ > drivers/gpu/drm/i915/display/intel_dsb.h | 2 ++ > 4 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 761be9fcaf10..99e6afda2db9 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -150,6 +150,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > return "GT_IRQ"; > case POWER_DOMAIN_DPLL_DC_OFF: > return "DPLL_DC_OFF"; > + case POWER_DOMAIN_DSB: > + return "DSB"; > default: > MISSING_CASE(domain); > return "?"; > @@ -2679,6 +2681,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > BIT_ULL(POWER_DOMAIN_AUX_C) | \ > + BIT_ULL(POWER_DOMAIN_DSB) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > > #define TGL_DDI_IO_D_TC1_POWER_DOMAINS ( \ > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index 2608a65af7fa..5e8136c65e02 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -77,6 +77,7 @@ enum intel_display_power_domain { > POWER_DOMAIN_GT_IRQ, > POWER_DOMAIN_DPLL_DC_OFF, > POWER_DOMAIN_INIT, > + POWER_DOMAIN_DSB, > > POWER_DOMAIN_NUM, > }; > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c > index ada006a690df..b47c31fa2551 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > @@ -103,16 +103,15 @@ intel_dsb_get(struct intel_crtc *crtc) > struct drm_i915_gem_object *obj; > struct i915_vma *vma; > u32 *buf; > - intel_wakeref_t wakeref; > > if (!HAS_DSB(i915)) > return dsb; > > + dsb->wakeref = intel_display_power_get(i915, POWER_DOMAIN_DSB); > + > if (dsb->refcount++ != 0) > return dsb; > > - wakeref = intel_runtime_pm_get(&i915->runtime_pm); > - > obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE); > if (IS_ERR(obj)) { > DRM_ERROR("Gem object creation failed\n"); > @@ -143,9 +142,6 @@ intel_dsb_get(struct intel_crtc *crtc) > * corresponding intel_dsb_put(): the important error message will > * already be logged above. > */ > - > - intel_runtime_pm_put(&i915->runtime_pm, wakeref); > - > return dsb; > } > > @@ -174,6 +170,8 @@ void intel_dsb_put(struct intel_dsb *dsb) > dsb->free_pos = 0; > dsb->ins_start_offset = 0; > } > + > + intel_display_power_put(i915, POWER_DOMAIN_DSB, dsb->wakeref); > } > > /** > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h > index 395ef9ce558e..ffb5afa935b7 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.h > +++ b/drivers/gpu/drm/i915/display/intel_dsb.h > @@ -9,6 +9,7 @@ > #include <linux/types.h> > > #include "i915_reg.h" > +#include "intel_wakeref.h" > > struct intel_crtc; > struct i915_vma; > @@ -26,6 +27,7 @@ struct intel_dsb { > enum dsb_id id; > u32 *cmd_buf; > struct i915_vma *vma; > + intel_wakeref_t wakeref; > > /* > * free_pos will point the first free entry position > -- > 2.23.0 >
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 761be9fcaf10..99e6afda2db9 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -150,6 +150,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) return "GT_IRQ"; case POWER_DOMAIN_DPLL_DC_OFF: return "DPLL_DC_OFF"; + case POWER_DOMAIN_DSB: + return "DSB"; default: MISSING_CASE(domain); return "?"; @@ -2679,6 +2681,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, BIT_ULL(POWER_DOMAIN_AUX_A) | \ BIT_ULL(POWER_DOMAIN_AUX_B) | \ BIT_ULL(POWER_DOMAIN_AUX_C) | \ + BIT_ULL(POWER_DOMAIN_DSB) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define TGL_DDI_IO_D_TC1_POWER_DOMAINS ( \ diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index 2608a65af7fa..5e8136c65e02 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -77,6 +77,7 @@ enum intel_display_power_domain { POWER_DOMAIN_GT_IRQ, POWER_DOMAIN_DPLL_DC_OFF, POWER_DOMAIN_INIT, + POWER_DOMAIN_DSB, POWER_DOMAIN_NUM, }; diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c index ada006a690df..b47c31fa2551 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.c +++ b/drivers/gpu/drm/i915/display/intel_dsb.c @@ -103,16 +103,15 @@ intel_dsb_get(struct intel_crtc *crtc) struct drm_i915_gem_object *obj; struct i915_vma *vma; u32 *buf; - intel_wakeref_t wakeref; if (!HAS_DSB(i915)) return dsb; + dsb->wakeref = intel_display_power_get(i915, POWER_DOMAIN_DSB); + if (dsb->refcount++ != 0) return dsb; - wakeref = intel_runtime_pm_get(&i915->runtime_pm); - obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE); if (IS_ERR(obj)) { DRM_ERROR("Gem object creation failed\n"); @@ -143,9 +142,6 @@ intel_dsb_get(struct intel_crtc *crtc) * corresponding intel_dsb_put(): the important error message will * already be logged above. */ - - intel_runtime_pm_put(&i915->runtime_pm, wakeref); - return dsb; } @@ -174,6 +170,8 @@ void intel_dsb_put(struct intel_dsb *dsb) dsb->free_pos = 0; dsb->ins_start_offset = 0; } + + intel_display_power_put(i915, POWER_DOMAIN_DSB, dsb->wakeref); } /** diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h index 395ef9ce558e..ffb5afa935b7 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.h +++ b/drivers/gpu/drm/i915/display/intel_dsb.h @@ -9,6 +9,7 @@ #include <linux/types.h> #include "i915_reg.h" +#include "intel_wakeref.h" struct intel_crtc; struct i915_vma; @@ -26,6 +27,7 @@ struct intel_dsb { enum dsb_id id; u32 *cmd_buf; struct i915_vma *vma; + intel_wakeref_t wakeref; /* * free_pos will point the first free entry position
There are reports of unexpected DSB busy/timeout happening after IGT tests finish running that apparently go away when the DMC firmware isn't loaded. The bspec doesn't say anything specific about DSB needing us to exit DC5/DC6, but let's try adding DSB usage to the "DC off" list and see if that changes the behavior. v2: Include intel_wakeref.h from intel_dsb.h to ensure the header stays self-contained. (CI) v3: Move intel_display_power_get() call earlier to cover cases where dsb->refcount is already non-zero and we return early. Also don't drop the wakeref at the end of the 'get' routine; wait until the 'put' for that, even in error conditions. (Swati) Cc: Swati Sharma <swati2.sharma@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by(v1): José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_display_power.c | 3 +++ drivers/gpu/drm/i915/display/intel_display_power.h | 1 + drivers/gpu/drm/i915/display/intel_dsb.c | 10 ++++------ drivers/gpu/drm/i915/display/intel_dsb.h | 2 ++ 4 files changed, 10 insertions(+), 6 deletions(-)