On Fri, Jan 05, 2024 at 11:09:37AM -0300, Gustavo Sousa wrote:
> Quoting Matt Roper (2024-01-04 20:52:32-03:00)
> >On Thu, Jan 04, 2024 at 03:48:34PM -0800, Matt Roper wrote:
> >> On Thu, Jan 04, 2024 at 12:21:50AM -0300, Gustavo Sousa wrote:
> >> > That's the function responsible for deriving that register's value; use
> >> > it.
> >> >
> >> > Signed-off-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
> >>
> >> Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> >
> >Forgot to mention...I think it's a bit jarring when the commit message
> >starts out referring to something in the headline ("That's the
> >function..."). It's probably a bit better to just re-state the function
> >name in the commit message again rather than assuming both get read
> >together.
>
> Thanks for the suggestion!
>
> I have sent a v2 updating the body of commit messages, not only for this one but
> also for two more patches. I have kept your r-b's. Let me know if that's okay.
Yep, the r-b's still apply. Thanks.
Matt
>
> --
> Gustavo Sousa
>
> >
> >
> >Matt
> >
> >>
> >> > ---
> >> > drivers/gpu/drm/i915/display/intel_cdclk.c | 26 +++-------------------
> >> > 1 file changed, 3 insertions(+), 23 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> > index fbe9aba41c35..26200ee3e23f 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> > @@ -2051,7 +2051,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> >> > static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> >> > {
> >> > u32 cdctl, expected;
> >> > - int cdclk, clock, vco;
> >> > + int cdclk, vco;
> >> >
> >> > intel_update_cdclk(dev_priv);
> >> > intel_cdclk_dump_config(dev_priv, &dev_priv->display.cdclk.hw, "Current CDCLK");
> >> > @@ -2076,6 +2076,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> >> > * so sanitize this register.
> >> > */
> >> > cdctl = intel_de_read(dev_priv, CDCLK_CTL);
> >> > + expected = bxt_cdclk_ctl(dev_priv, &dev_priv->display.cdclk.hw, INVALID_PIPE);
> >> >
> >> > /*
> >> > * Let's ignore the pipe field, since BIOS could have configured the
> >> > @@ -2083,28 +2084,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> >> > * (PIPE_NONE).
> >> > */
> >> > cdctl &= ~bxt_cdclk_cd2x_pipe(dev_priv, INVALID_PIPE);
> >> > -
> >> > - if (DISPLAY_VER(dev_priv) >= 20)
> >> > - expected = MDCLK_SOURCE_SEL_CDCLK_PLL;
> >> > - else
> >> > - expected = skl_cdclk_decimal(cdclk);
> >> > -
> >> > - /* Figure out what CD2X divider we should be using for this cdclk */
> >> > - if (HAS_CDCLK_SQUASH(dev_priv))
> >> > - clock = dev_priv->display.cdclk.hw.vco / 2;
> >> > - else
> >> > - clock = dev_priv->display.cdclk.hw.cdclk;
> >> > -
> >> > - expected |= bxt_cdclk_cd2x_div_sel(dev_priv, clock,
> >> > - dev_priv->display.cdclk.hw.vco);
> >> > -
> >> > - /*
> >> > - * Disable SSA Precharge when CD clock frequency < 500 MHz,
> >> > - * enable otherwise.
> >> > - */
> >> > - if ((IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) &&
> >> > - dev_priv->display.cdclk.hw.cdclk >= 500000)
> >> > - expected |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> >> > + expected &= ~bxt_cdclk_cd2x_pipe(dev_priv, INVALID_PIPE);
> >> >
> >> > if (cdctl == expected)
> >> > /* All well; nothing to sanitize */
> >> > --
> >> > 2.43.0
> >> >
> >>
> >> --
> >> Matt Roper
> >> Graphics Software Engineer
> >> Linux GPU Platform Enablement
> >> Intel Corporation
> >
> >--
> >Matt Roper
> >Graphics Software Engineer
> >Linux GPU Platform Enablement
> >Intel Corporation
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
