Message ID | 20181221171436.8218-3-ville.syrjala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | skl+ watermark stuff | expand |
On Fri, 2018-12-21 at 19:14 +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I thought we could remove all the early latency==0 checks > and rely on skl_wm_method{1,2}() checking for it. But > skl_compute_plane_wm() applies a bunch of workarounds to bump > up the latency before calling those guys so clearly it won't > end up doing the right thing. Also not sure if the calculations > based on the method1/2 results are safe agaisnt overflows so > it might not work all that well in any case. Let's put the > early check back. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index d132ef10fa60..0aac7e7b660f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct > intel_crtc_state *cstate, > to_intel_atomic_state(cstate->base.state); > bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state); > > + if (latency == 0) > + return; > + > /* Display WA #1141: kbl,cfl */ > if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || > IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) && Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
On Fri, Dec 21, 2018 at 07:14:29PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I thought we could remove all the early latency==0 checks > and rely on skl_wm_method{1,2}() checking for it. But > skl_compute_plane_wm() applies a bunch of workarounds to bump > up the latency before calling those guys so clearly it won't > end up doing the right thing. Also not sure if the calculations > based on the method1/2 results are safe agaisnt overflows so > it might not work all that well in any case. Let's put the > early check back. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Should we remove the tests from skl_wm_method{1,2}() now? I suppose someone could still use the debugfs interface to set a latency value of exactly (UINT32_MAX - workaround amount) to make latency wrap around and hit 0, but I'm not sure if that's really any worse than if they shoot themselves in the foot by setting a too-low non-zero latency. I don't think we divide by latency anywhere. Either way, Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index d132ef10fa60..0aac7e7b660f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate, > to_intel_atomic_state(cstate->base.state); > bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state); > > + if (latency == 0) > + return; > + > /* Display WA #1141: kbl,cfl */ > if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || > IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) && > -- > 2.19.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jan 29, 2019 at 03:54:30PM -0800, Matt Roper wrote: > On Fri, Dec 21, 2018 at 07:14:29PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > I thought we could remove all the early latency==0 checks > > and rely on skl_wm_method{1,2}() checking for it. But > > skl_compute_plane_wm() applies a bunch of workarounds to bump > > up the latency before calling those guys so clearly it won't > > end up doing the right thing. Also not sure if the calculations > > based on the method1/2 results are safe agaisnt overflows so > > it might not work all that well in any case. Let's put the > > early check back. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Should we remove the tests from skl_wm_method{1,2}() now? I suppose > someone could still use the debugfs interface to set a latency value of > exactly (UINT32_MAX - workaround amount) to make latency wrap around and > hit 0, but I'm not sure if that's really any worse than if they shoot > themselves in the foot by setting a too-low non-zero latency. I don't > think we divide by latency anywhere. I'd probably prefer to fix the overflows somehow so that the code could work the same way as the pre-skl code (ie. just check for 0 latency in the method1/2 functions). But that would require some actual thinking. > > Either way, > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Thanks for the reviews. Series pushed to dinq. > > > > --- > > drivers/gpu/drm/i915/intel_pm.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index d132ef10fa60..0aac7e7b660f 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate, > > to_intel_atomic_state(cstate->base.state); > > bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state); > > > > + if (latency == 0) > > + return; > > + > > /* Display WA #1141: kbl,cfl */ > > if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || > > IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) && > > -- > > 2.19.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d132ef10fa60..0aac7e7b660f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate, to_intel_atomic_state(cstate->base.state); bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state); + if (latency == 0) + return; + /* Display WA #1141: kbl,cfl */ if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&