| Message ID | 20200608065552.21728-1-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Revert "drm/i915: Remove unneeded hack now for CDCLK" | expand |
On Mon, 08 Jun 2020, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote: > This reverts commit 82ea174dc5425d4e85e25d0c4ba961a2e494392a. > Please explain why. What's going on, why we need the revert. It's fine to reply here, the commit message can be amended by whoever applies the patch. BR, Jani. > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > Fixes: cd1915460861 ("drm/i915: Adjust CDCLK accordingly to our DBuf bw needs") > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 08468b121d02..45f7f33d1144 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -2071,6 +2071,18 @@ 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); > > + /* > + * HACK. 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. > + */ > + if (IS_TIGERLAKE(dev_priv)) > + min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate); > + > if (min_cdclk > dev_priv->max_cdclk_freq) { > drm_dbg_kms(&dev_priv->drm, > "required cdclk (%d kHz) exceeds max (%d kHz)\n",
On Mon, Jun 08, 2020 at 11:21:14AM +0300, Jani Nikula wrote: > On Mon, 08 Jun 2020, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote: > > This reverts commit 82ea174dc5425d4e85e25d0c4ba961a2e494392a. > > > > Please explain why. What's going on, why we need the revert. > > It's fine to reply here, the commit message can be amended by whoever > applies the patch. Yes, Unfortunately according to our recent findings there is still some unidentified factor, requiring CDCLK to be set higher - otherwise we still get underruns on some multipipe configurations, despite CDCLK being set according to BSpec formula. So getting again back into debug mode to indentify the cause, meanwhile setting CDCLK=Pixel rate back in order to remove regression in 10% of the cases due to FIFO underruns. Stan > > BR, > Jani. > > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > Fixes: cd1915460861 ("drm/i915: Adjust CDCLK accordingly to our DBuf bw needs") > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index 08468b121d02..45f7f33d1144 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -2071,6 +2071,18 @@ 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); > > > > + /* > > + * HACK. 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. > > + */ > > + if (IS_TIGERLAKE(dev_priv)) > > + min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate); > > + > > if (min_cdclk > dev_priv->max_cdclk_freq) { > > drm_dbg_kms(&dev_priv->drm, > > "required cdclk (%d kHz) exceeds max (%d kHz)\n", > > -- > Jani Nikula, Intel Open Source Graphics Center
On Mon, 08 Jun 2020, "Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com> wrote: > On Mon, Jun 08, 2020 at 11:21:14AM +0300, Jani Nikula wrote: >> On Mon, 08 Jun 2020, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote: >> > This reverts commit 82ea174dc5425d4e85e25d0c4ba961a2e494392a. >> > >> >> Please explain why. What's going on, why we need the revert. >> >> It's fine to reply here, the commit message can be amended by whoever >> applies the patch. > > Yes, > > Unfortunately according to our recent findings there is still some > unidentified factor, requiring CDCLK to be set higher - otherwise we > still get underruns on some multipipe configurations, despite CDCLK being set > according to BSpec formula. So getting again back into debug mode to > indentify the cause, meanwhile setting CDCLK=Pixel rate back in order > to remove regression in 10% of the cases due to FIFO underruns. Thanks, pushed. BR, Jani. > > Stan > >> >> BR, >> Jani. >> >> >> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> >> > Fixes: cd1915460861 ("drm/i915: Adjust CDCLK accordingly to our DBuf bw needs") >> > --- >> > drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ++++++++++++ >> > 1 file changed, 12 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c >> > index 08468b121d02..45f7f33d1144 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c >> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c >> > @@ -2071,6 +2071,18 @@ 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); >> > >> > + /* >> > + * HACK. 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. >> > + */ >> > + if (IS_TIGERLAKE(dev_priv)) >> > + min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate); >> > + >> > if (min_cdclk > dev_priv->max_cdclk_freq) { >> > drm_dbg_kms(&dev_priv->drm, >> > "required cdclk (%d kHz) exceeds max (%d kHz)\n", >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 08468b121d02..45f7f33d1144 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2071,6 +2071,18 @@ 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); + /* + * HACK. 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. + */ + if (IS_TIGERLAKE(dev_priv)) + min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate); + if (min_cdclk > dev_priv->max_cdclk_freq) { drm_dbg_kms(&dev_priv->drm, "required cdclk (%d kHz) exceeds max (%d kHz)\n",
This reverts commit 82ea174dc5425d4e85e25d0c4ba961a2e494392a. Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Fixes: cd1915460861 ("drm/i915: Adjust CDCLK accordingly to our DBuf bw needs") --- drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)