| Message ID | 20161124040135.5517-3-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Mahesh Kumar schreef op do 24-11-2016 om 09:31 [+0530]: > If IPC is enabled in BXT, display underruns are observed. > WA: The Line Time programmed in the WM_LINETIME register should be > half of the actual calculated Line Time. > > Programmed Line Time = 1/2*Calculated Line Time > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++-- > 3 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > index 445fec9..1b0a589 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1243,6 +1243,8 @@ 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; > + > /* Everything is in place, we can now relax! */ > DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n", > driver.name, driver.major, driver.minor, > driver.patchlevel, > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index 63c0ea0..6691a4e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2128,6 +2128,8 @@ struct drm_i915_private { > /* perform PHY state sanity checks? */ > bool chv_phy_assert[2]; > > + bool ipc_enabled; > + > /* Used to save the pipe-to-encoder mapping for audio */ > struct intel_encoder *av_enc_map[I915_MAX_PIPES]; > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 8908736..7090a7c 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3769,7 +3769,10 @@ skl_compute_wm_level(const struct > drm_i915_private *dev_priv, > static uint32_t > skl_compute_linetime_wm(struct intel_crtc_state *cstate) > { > + struct drm_atomic_state *state = cstate->base.state; > + struct drm_i915_private *dev_priv = to_i915(state->dev); > uint32_t pixel_rate; > + uint32_t linetime_wm; > > if (!cstate->base.active) > return 0; > @@ -3779,8 +3782,12 @@ skl_compute_linetime_wm(struct > intel_crtc_state *cstate) > if (WARN_ON(pixel_rate == 0)) > return 0; > > - return DIV_ROUND_UP(8 * cstate- > >base.adjusted_mode.crtc_htotal * 1000, > - pixel_rate); > + linetime_wm = DIV_ROUND_UP(8 * cstate- > >base.adjusted_mode.crtc_htotal * > + 1000, pixel_rate); > + if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled) > + linetime_wm = DIV_ROUND_UP(linetime_wm, 2); I've asked on irc to be sure, but this needs slightly more info in the code as well. Same for 3/8 and 8/8. 12:47 < mlankhorst> danvet: if we add hw workarounds that don't have a name, do we still have to write comments about them in the code? 12:48 < danvet> at least a bspec reference 12:48 < danvet> some hint to make it possible to find where it's from 12:49 < danvet> e.g. mesa has traditionally just pasted the entire bspec paragraph 12:49 < mlankhorst> ok 12:49 < danvet> we've done that too in the past, but then the wa db mostly made that redundant 12:49 < danvet> but if there's no wa entry, then full deal 12:49 < danvet> also, maybe a bspec bug notice to pls add one 12:49 < danvet> you should be able to do that over the web, and afaik there should be one for every w/a --------------------------------------------------------------------- Intel International B.V. Registered in The Netherlands under number 34098535 Statutory seat: Haarlemmermeer Registered address: Capronilaan 37, 1119NG Schiphol-Rijk This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
Hi, Will keep WA number in commit message/WA location. thanks, Regards, -Mahesh On Thursday 24 November 2016 06:21 PM, Lankhorst, Maarten wrote: > Mahesh Kumar schreef op do 24-11-2016 om 09:31 [+0530]: >> If IPC is enabled in BXT, display underruns are observed. >> WA: The Line Time programmed in the WM_LINETIME register should be >> half of the actual calculated Line Time. >> >> Programmed Line Time = 1/2*Calculated Line Time >> >> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> >> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 2 ++ >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ >> drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++-- >> 3 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c >> b/drivers/gpu/drm/i915/i915_drv.c >> index 445fec9..1b0a589 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -1243,6 +1243,8 @@ 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; >> + >> /* Everything is in place, we can now relax! */ >> DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n", >> driver.name, driver.major, driver.minor, >> driver.patchlevel, >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 63c0ea0..6691a4e 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2128,6 +2128,8 @@ struct drm_i915_private { >> /* perform PHY state sanity checks? */ >> bool chv_phy_assert[2]; >> >> + bool ipc_enabled; >> + >> /* Used to save the pipe-to-encoder mapping for audio */ >> struct intel_encoder *av_enc_map[I915_MAX_PIPES]; >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> index 8908736..7090a7c 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3769,7 +3769,10 @@ skl_compute_wm_level(const struct >> drm_i915_private *dev_priv, >> static uint32_t >> skl_compute_linetime_wm(struct intel_crtc_state *cstate) >> { >> + struct drm_atomic_state *state = cstate->base.state; >> + struct drm_i915_private *dev_priv = to_i915(state->dev); >> uint32_t pixel_rate; >> + uint32_t linetime_wm; >> >> if (!cstate->base.active) >> return 0; >> @@ -3779,8 +3782,12 @@ skl_compute_linetime_wm(struct >> intel_crtc_state *cstate) >> if (WARN_ON(pixel_rate == 0)) >> return 0; >> >> - return DIV_ROUND_UP(8 * cstate- >>> base.adjusted_mode.crtc_htotal * 1000, >> - pixel_rate); >> + linetime_wm = DIV_ROUND_UP(8 * cstate- >>> base.adjusted_mode.crtc_htotal * >> + 1000, pixel_rate); >> + if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled) >> + linetime_wm = DIV_ROUND_UP(linetime_wm, 2); > I've asked on irc to be sure, but this needs slightly more info in the > code as well. Same for 3/8 and 8/8. > > 12:47 < mlankhorst> danvet: if we add hw workarounds that don't have a > name, do we still have to write comments about them in the code? > 12:48 < danvet> at least a bspec reference > 12:48 < danvet> some hint to make it possible to find where it's from > 12:49 < danvet> e.g. mesa has traditionally just pasted the entire > bspec paragraph > 12:49 < mlankhorst> ok > 12:49 < danvet> we've done that too in the past, but then the wa db > mostly made that redundant > 12:49 < danvet> but if there's no wa entry, then full deal > 12:49 < danvet> also, maybe a bspec bug notice to pls add one > 12:49 < danvet> you should be able to do that over the web, and afaik > there should be one for every w/a
Hey, Mahesh Kumar schreef op ma 28-11-2016 om 18:37 [+0530]: > Hi, > > Will keep WA number in commit message/WA location. > thanks, Sounds good, with that fixed patches 1-5 and 7 look good to me. I think patch 6 will no longer be required since the workaround status will also be kept inside intel_state. ~Maarten --------------------------------------------------------------------- Intel International B.V. Registered in The Netherlands under number 34098535 Statutory seat: Haarlemmermeer Registered address: Capronilaan 37, 1119NG Schiphol-Rijk This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 445fec9..1b0a589 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1243,6 +1243,8 @@ 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; + /* Everything is in place, we can now relax! */ DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n", driver.name, driver.major, driver.minor, driver.patchlevel, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 63c0ea0..6691a4e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2128,6 +2128,8 @@ struct drm_i915_private { /* perform PHY state sanity checks? */ bool chv_phy_assert[2]; + bool ipc_enabled; + /* Used to save the pipe-to-encoder mapping for audio */ struct intel_encoder *av_enc_map[I915_MAX_PIPES]; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8908736..7090a7c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3769,7 +3769,10 @@ skl_compute_wm_level(const struct drm_i915_private *dev_priv, static uint32_t skl_compute_linetime_wm(struct intel_crtc_state *cstate) { + struct drm_atomic_state *state = cstate->base.state; + struct drm_i915_private *dev_priv = to_i915(state->dev); uint32_t pixel_rate; + uint32_t linetime_wm; if (!cstate->base.active) return 0; @@ -3779,8 +3782,12 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate) if (WARN_ON(pixel_rate == 0)) return 0; - return DIV_ROUND_UP(8 * cstate->base.adjusted_mode.crtc_htotal * 1000, - pixel_rate); + linetime_wm = DIV_ROUND_UP(8 * cstate->base.adjusted_mode.crtc_htotal * + 1000, pixel_rate); + if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled) + linetime_wm = DIV_ROUND_UP(linetime_wm, 2); + + return linetime_wm; } static void skl_compute_transition_wm(struct intel_crtc_state *cstate,