Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix CD2X pipe select masking during cdclk sanitation
HI, > -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of > Chris > Wilson > Sent: keskiviikko 11. syyskuuta 2019 18.09 > To: Roper, Matthew D ; Ville Syrjala > > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix CD2X pipe select masking > during > cdclk sanitation > > Quoting Matt Roper (2019-09-11 16:00:44) > > I'm confused why pre-merge CI flagged the results as a success if TGL > > was hitting this? > > We've only just (Fri) got CI's Tigerlake surviving boot, so a second failure > during boot > would have been missed. For the summary report, Tigerlake is currently > suppressed > as it is not yet proven itself as being a stable result. (Which is a bit of a > nuisance as > you have to remind yourself to actually check the details.) -Chris Exactly. > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix CD2X pipe select masking during cdclk sanitation
HI,
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> Matt
> Roper
> Sent: keskiviikko 11. syyskuuta 2019 18.01
> To: Ville Syrjala
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix CD2X pipe select masking
> during
> cdclk sanitation
>
> On Wed, Sep 11, 2019 at 04:31:27PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > We're forgetting to mask off all three pipe select bits from the
> > CDCLK_CTL value on icl+ which may lead to the extra bit being left in.
> > That will cause us to consider the current hardware cdclk state as
> > invalid, and we proceed to sanitize it even though the hardware may
> > have active pipes and whatnot.
> >
> > Fix up the mask so we get rid of all three pipe select bits and thus
> > hopefully no longer sanitize cdclk when it's already correctly
> > programmed.
> >
> > Cc: Matt Roper
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111641
> > Fixes: 0c1279b58fc7 ("drm/i915: Consolidate {bxt,cnl,icl}_init_cdclk")
> > Signed-off-by: Ville Syrjälä
>
> Reviewed-by: Matt Roper
>
> I'm confused why pre-merge CI flagged the results as a success if TGL was
> hitting
> this?
It died also on CI: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14345/?
Martin, Tomi can you explain?
>
>
> Matt
>
> > ---
> > drivers/gpu/drm/i915/display/intel_cdclk.c | 42
> > --
> > 1 file changed, 23 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 6b75d2a91cd9..f59a6f775177 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -1464,6 +1464,26 @@ static void cnl_cdclk_pll_enable(struct
> drm_i915_private *dev_priv, int vco)
> > dev_priv->cdclk.hw.vco = vco;
> > }
> >
> > +static u32 bxt_cdclk_cd2x_pipe(struct drm_i915_private *dev_priv,
> > +enum pipe pipe) {
> > + if (INTEL_GEN(dev_priv) >= 12) {
> > + if (pipe == INVALID_PIPE)
> > + return TGL_CDCLK_CD2X_PIPE_NONE;
> > + else
> > + return TGL_CDCLK_CD2X_PIPE(pipe);
> > + } else if (INTEL_GEN(dev_priv) >= 11) {
> > + if (pipe == INVALID_PIPE)
> > + return ICL_CDCLK_CD2X_PIPE_NONE;
> > + else
> > + return ICL_CDCLK_CD2X_PIPE(pipe);
> > + } else {
> > + if (pipe == INVALID_PIPE)
> > + return BXT_CDCLK_CD2X_PIPE_NONE;
> > + else
> > + return BXT_CDCLK_CD2X_PIPE(pipe);
> > + }
> > +}
> > +
> > static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> > const struct intel_cdclk_state *cdclk_state,
> > enum pipe pipe)
> > @@ -1534,24 +1554,8 @@ static void bxt_set_cdclk(struct drm_i915_private
> *dev_priv,
> > bxt_de_pll_enable(dev_priv, vco);
> > }
> >
> > - val = divider | skl_cdclk_decimal(cdclk);
> > -
> > - if (INTEL_GEN(dev_priv) >= 12) {
> > - if (pipe == INVALID_PIPE)
> > - val |= TGL_CDCLK_CD2X_PIPE_NONE;
> > - else
> > - val |= TGL_CDCLK_CD2X_PIPE(pipe);
> > - } else if (INTEL_GEN(dev_priv) >= 11) {
> > - if (pipe == INVALID_PIPE)
> > - val |= ICL_CDCLK_CD2X_PIPE_NONE;
> > - else
> > - val |= ICL_CDCLK_CD2X_PIPE(pipe);
> > - } else {
> > - if (pipe == INVALID_PIPE)
> > - val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > - else
> > - val |= BXT_CDCLK_CD2X_PIPE(pipe);
> > - }
> > + val = divider | skl_cdclk_decimal(cdclk) |
> > + bxt_cdclk_cd2x_pipe(dev_priv, pipe);
> >
> > /*
> > * Disable SSA Precharge when CD clock frequency < 500 MHz, @@
> > -1620,7 +1624,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private
> *dev_priv)
> > * dividers both synching to an active pipe, or asynchronously
> > * (PIPE_NONE).
> > */
> > - cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
> > + cdctl &= ~bxt_cdclk_cd2x_pipe(dev_priv, INVALID_PIPE);
> >
> > /* Make sure this is a legal cdclk value for the platform */
> > cdclk = bxt_calc_cdclk(dev_priv, dev_priv->cdclk.hw.cdclk);
> > --
> > 2.21.0
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix CD2X pipe select masking during cdclk sanitation
On Wed, 11 Sep 2019, Matt Roper wrote: > I'm confused why pre-merge CI flagged the results as a success if TGL > was hitting this? I didn't check the specifics, but the full set of IGT tests is only run on a limited number of platforms, and TGL is not yet one of them. You get the narrow range of tests on a wide range of platforms and the wide range of tests on a narrow range of platforms. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix CD2X pipe select masking during cdclk sanitation
Quoting Matt Roper (2019-09-11 16:00:44) > I'm confused why pre-merge CI flagged the results as a success if TGL > was hitting this? We've only just (Fri) got CI's Tigerlake surviving boot, so a second failure during boot would have been missed. For the summary report, Tigerlake is currently suppressed as it is not yet proven itself as being a stable result. (Which is a bit of a nuisance as you have to remind yourself to actually check the details.) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix CD2X pipe select masking during cdclk sanitation
On Wed, Sep 11, 2019 at 04:31:27PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> We're forgetting to mask off all three pipe select bits from the
> CDCLK_CTL value on icl+ which may lead to the extra bit being
> left in. That will cause us to consider the current hardware
> cdclk state as invalid, and we proceed to sanitize it even
> though the hardware may have active pipes and whatnot.
>
> Fix up the mask so we get rid of all three pipe select bits
> and thus hopefully no longer sanitize cdclk when it's already
> correctly programmed.
>
> Cc: Matt Roper
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111641
> Fixes: 0c1279b58fc7 ("drm/i915: Consolidate {bxt,cnl,icl}_init_cdclk")
> Signed-off-by: Ville Syrjälä
Reviewed-by: Matt Roper
I'm confused why pre-merge CI flagged the results as a success if TGL
was hitting this?
Matt
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 42 --
> 1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 6b75d2a91cd9..f59a6f775177 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1464,6 +1464,26 @@ static void cnl_cdclk_pll_enable(struct
> drm_i915_private *dev_priv, int vco)
> dev_priv->cdclk.hw.vco = vco;
> }
>
> +static u32 bxt_cdclk_cd2x_pipe(struct drm_i915_private *dev_priv, enum pipe
> pipe)
> +{
> + if (INTEL_GEN(dev_priv) >= 12) {
> + if (pipe == INVALID_PIPE)
> + return TGL_CDCLK_CD2X_PIPE_NONE;
> + else
> + return TGL_CDCLK_CD2X_PIPE(pipe);
> + } else if (INTEL_GEN(dev_priv) >= 11) {
> + if (pipe == INVALID_PIPE)
> + return ICL_CDCLK_CD2X_PIPE_NONE;
> + else
> + return ICL_CDCLK_CD2X_PIPE(pipe);
> + } else {
> + if (pipe == INVALID_PIPE)
> + return BXT_CDCLK_CD2X_PIPE_NONE;
> + else
> + return BXT_CDCLK_CD2X_PIPE(pipe);
> + }
> +}
> +
> static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> const struct intel_cdclk_state *cdclk_state,
> enum pipe pipe)
> @@ -1534,24 +1554,8 @@ static void bxt_set_cdclk(struct drm_i915_private
> *dev_priv,
> bxt_de_pll_enable(dev_priv, vco);
> }
>
> - val = divider | skl_cdclk_decimal(cdclk);
> -
> - if (INTEL_GEN(dev_priv) >= 12) {
> - if (pipe == INVALID_PIPE)
> - val |= TGL_CDCLK_CD2X_PIPE_NONE;
> - else
> - val |= TGL_CDCLK_CD2X_PIPE(pipe);
> - } else if (INTEL_GEN(dev_priv) >= 11) {
> - if (pipe == INVALID_PIPE)
> - val |= ICL_CDCLK_CD2X_PIPE_NONE;
> - else
> - val |= ICL_CDCLK_CD2X_PIPE(pipe);
> - } else {
> - if (pipe == INVALID_PIPE)
> - val |= BXT_CDCLK_CD2X_PIPE_NONE;
> - else
> - val |= BXT_CDCLK_CD2X_PIPE(pipe);
> - }
> + val = divider | skl_cdclk_decimal(cdclk) |
> + bxt_cdclk_cd2x_pipe(dev_priv, pipe);
>
> /*
>* Disable SSA Precharge when CD clock frequency < 500 MHz,
> @@ -1620,7 +1624,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private
> *dev_priv)
>* dividers both synching to an active pipe, or asynchronously
>* (PIPE_NONE).
>*/
> - cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
> + cdctl &= ~bxt_cdclk_cd2x_pipe(dev_priv, INVALID_PIPE);
>
> /* Make sure this is a legal cdclk value for the platform */
> cdclk = bxt_calc_cdclk(dev_priv, dev_priv->cdclk.hw.cdclk);
> --
> 2.21.0
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
