On Wed, Jan 08, 2020 at 02:26:50PM +0200, Stanislav Lisovskiy wrote: > There seems to be some undocumented bandwidth > bottleneck/dependency which scales with CDCLK, > causing FIFO underruns when CDCLK is too low, > even when it's correct from BSpec point of view. > > Currently for TGL platforms we calculate > min_cdclk initially based on pixel_rate divided > by 2, accounting for also plane requirements, > however in some cases the lowest possible CDCLK > doesn't work and causing the underruns. > > Explicitly stating here that this seems to be currently > rather a Hack, than final solution. > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > Bugzilla: https://gitlab.freedesktop.org/drm/intel/issues/402 > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 7d1ab1e5b7c3..3db4060ed190 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -2004,6 +2004,13 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) > /* Account for additional needs from the planes */ > min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk); > > + if (IS_GEN(dev_priv, 12)) { > + if (min_cdclk <= DIV_ROUND_UP(crtc_state->pixel_rate, 2)) { > + min_cdclk = min(min_cdclk * 2, > + ((int)dev_priv->max_cdclk_freq)); > + } min_cdclk is initially set to DIV_ROUND_UP(crtc_state->pixel_rate, 2), and then only potentially increases from there, so we're really just checking for equality here, right (the "<" case is impossible)? Should we worry that one of the other checks (audio, planes, etc.) might have just slightly bumped up min_cdclk, causing this condition to fail, but not bumping it far enough to get us into the seemingly-safe zone? Maybe it would be simpler/safer to just do something like /* HACK */ if (IS_TIGERLAKE(dev_priv)) min_cdclk = clamp(min_cdclk, crtc_state->pixel_rate, dev_priv->max_cdclk_freq); which seems closer to the true goal of the workaround? Regardless, we should probably also have a code comment on whatever we come up with just like we do on all the other min_cdclk adjustments, especially since this one is a hack that doesn't actually match the bspec. Matt > + } > + > if (min_cdclk > dev_priv->max_cdclk_freq) { > DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n", > min_cdclk, dev_priv->max_cdclk_freq); > -- > 2.24.1.485.gad05a3d8e5 > -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
