Re: [PATCH 6/8] drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes
On Mon, Mar 11, 2024 at 06:13:29PM -0300, Gustavo Sousa wrote: > Quoting Lisovskiy, Stanislav (2024-03-11 18:01:04-03:00) > >On Mon, Mar 04, 2024 at 03:30:25PM -0300, Gustavo Sousa wrote: > >> Commit 394b4b7df9f7 ("drm/i915/lnl: Add CDCLK table") and commit > >> 3d3696c0fed1 ("drm/i915/lnl: Start using CDCLK through PLL") started > >> adding support for CDCLK programming support for Xe2LPD. One final piece > >> is missing, which is the programming necessary for changed in the ratio > >> between MDCLK and CDCLK. Let's do that now. > >> > >> BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the > >> ratio between MDCLK and CDCLK changes. The updates must be done before > >> changing the CDCLK when decreasing the frequency; or after it when > >> increasing the frequency. > >> > >> Ratio-related updates to MBUS_CTL also depend on the state of MBus > >> joining, so they are performed by either CDCLK change sequence or by > >> changes in MBus joining. Since one might happen independently of the > >> other, we need to make sure that both logics see the necessary state > >> values when programming that register. MBus joining logic needs to know > >> the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio > >> field of struct intel_dbuf_state. > >> > >> For the CDCLK logic, we need to have something similar: we need to > >> propagate the status of MBus joining to struct intel_cdclk_state. Do > >> that by adding the field joined_mbus to struct intel_cdclk_config. > >> (Preferably, that field would be added to intel_cdclk_state, however > >> currently only intel_cdclk_config is passed down to the functions that > >> do the register programming. We might revisit this decision if we find > >> that refactoring the code to pass the whole intel_cdclk_state is worth > >> it.) > >> > >> Bspec: 68864, 68868, 69090, 69482 > >> Signed-off-by: Gustavo Sousa > >> --- > >> drivers/gpu/drm/i915/display/intel_cdclk.c| 31 ++ > >> drivers/gpu/drm/i915/display/intel_cdclk.h| 3 ++ > >> drivers/gpu/drm/i915/display/skl_watermark.c | 40 +++ > >> drivers/gpu/drm/i915/display/skl_watermark.h | 1 + > >> .../gpu/drm/i915/display/skl_watermark_regs.h | 18 + > >> 5 files changed, 77 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > >> b/drivers/gpu/drm/i915/display/intel_cdclk.c > >> index 04a6e9806254..12753589072d 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > >> @@ -40,6 +40,7 @@ > >> #include "intel_psr.h" > >> #include "intel_vdsc.h" > >> #include "skl_watermark.h" > >> +#include "skl_watermark_regs.h" > >> #include "vlv_sideband.h" > >> > >> /** > >> @@ -1683,6 +1684,8 @@ static void bxt_get_cdclk(struct drm_i915_private > >> *dev_priv, > >> } > >> > >> out: > >> +if (DISPLAY_VER(dev_priv) >= 20) > >> +cdclk_config->joined_mbus = intel_de_read(dev_priv, > >> MBUS_CTL) & MBUS_JOIN; > >> /* > >> * Can't read this out :( Let's assume it's > >> * at least what the CDCLK frequency requires. > >> @@ -1908,6 +1911,14 @@ u8 intel_mdclk_cdclk_ratio(struct drm_i915_private > >> *i915, > >> } > >> } > >> > >> +static void xe2lpd_mdclk_cdclk_ratio_program(struct drm_i915_private > >> *i915, > >> + const struct > >> intel_cdclk_config *cdclk_config) > >> +{ > >> +intel_dbuf_mdclk_cdclk_ratio_update(i915, > >> +intel_mdclk_cdclk_ratio(i915, > >> cdclk_config), > >> +cdclk_config->joined_mbus); > >> +} > >> + > >> static bool cdclk_compute_crawl_and_squash_midpoint(struct > >> drm_i915_private *i915, > >> const struct > >> intel_cdclk_config *old_cdclk_config, > >> const struct > >> intel_cdclk_config *new_cdclk_config, > >> @@ -2089,6 +2100,9 @@ static void bxt_set_cdclk(struct drm_i915_private > >> *dev_priv, > >> return; > >> } > >> > >> +if (DISPLAY_VER(dev_priv) >= 20 && cdclk < > >> dev_priv->display.cdclk.hw.cdclk) > >> +xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config); > >> + > >> if (cdclk_compute_crawl_and_squash_midpoint(dev_priv, > >> _priv->display.cdclk.hw, > >> cdclk_config, > >> _cdclk_config)) { > >> _bxt_set_cdclk(dev_priv, _cdclk_config, pipe); > >> @@ -2097,6 +2111,9 @@ static void bxt_set_cdclk(struct drm_i915_private > >> *dev_priv, > >> _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > >> } > >> > >> +if (DISPLAY_VER(dev_priv) >= 20 && cdclk > > >> dev_priv->display.cdclk.hw.cdclk) > >> +
Re: [PATCH 6/8] drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes
Quoting Lisovskiy, Stanislav (2024-03-11 18:01:04-03:00) >On Mon, Mar 04, 2024 at 03:30:25PM -0300, Gustavo Sousa wrote: >> Commit 394b4b7df9f7 ("drm/i915/lnl: Add CDCLK table") and commit >> 3d3696c0fed1 ("drm/i915/lnl: Start using CDCLK through PLL") started >> adding support for CDCLK programming support for Xe2LPD. One final piece >> is missing, which is the programming necessary for changed in the ratio >> between MDCLK and CDCLK. Let's do that now. >> >> BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the >> ratio between MDCLK and CDCLK changes. The updates must be done before >> changing the CDCLK when decreasing the frequency; or after it when >> increasing the frequency. >> >> Ratio-related updates to MBUS_CTL also depend on the state of MBus >> joining, so they are performed by either CDCLK change sequence or by >> changes in MBus joining. Since one might happen independently of the >> other, we need to make sure that both logics see the necessary state >> values when programming that register. MBus joining logic needs to know >> the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio >> field of struct intel_dbuf_state. >> >> For the CDCLK logic, we need to have something similar: we need to >> propagate the status of MBus joining to struct intel_cdclk_state. Do >> that by adding the field joined_mbus to struct intel_cdclk_config. >> (Preferably, that field would be added to intel_cdclk_state, however >> currently only intel_cdclk_config is passed down to the functions that >> do the register programming. We might revisit this decision if we find >> that refactoring the code to pass the whole intel_cdclk_state is worth >> it.) >> >> Bspec: 68864, 68868, 69090, 69482 >> Signed-off-by: Gustavo Sousa >> --- >> drivers/gpu/drm/i915/display/intel_cdclk.c| 31 ++ >> drivers/gpu/drm/i915/display/intel_cdclk.h| 3 ++ >> drivers/gpu/drm/i915/display/skl_watermark.c | 40 +++ >> drivers/gpu/drm/i915/display/skl_watermark.h | 1 + >> .../gpu/drm/i915/display/skl_watermark_regs.h | 18 + >> 5 files changed, 77 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c >> b/drivers/gpu/drm/i915/display/intel_cdclk.c >> index 04a6e9806254..12753589072d 100644 >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c >> @@ -40,6 +40,7 @@ >> #include "intel_psr.h" >> #include "intel_vdsc.h" >> #include "skl_watermark.h" >> +#include "skl_watermark_regs.h" >> #include "vlv_sideband.h" >> >> /** >> @@ -1683,6 +1684,8 @@ static void bxt_get_cdclk(struct drm_i915_private >> *dev_priv, >> } >> >> out: >> +if (DISPLAY_VER(dev_priv) >= 20) >> +cdclk_config->joined_mbus = intel_de_read(dev_priv, >> MBUS_CTL) & MBUS_JOIN; >> /* >> * Can't read this out :( Let's assume it's >> * at least what the CDCLK frequency requires. >> @@ -1908,6 +1911,14 @@ u8 intel_mdclk_cdclk_ratio(struct drm_i915_private >> *i915, >> } >> } >> >> +static void xe2lpd_mdclk_cdclk_ratio_program(struct drm_i915_private *i915, >> + const struct >> intel_cdclk_config *cdclk_config) >> +{ >> +intel_dbuf_mdclk_cdclk_ratio_update(i915, >> +intel_mdclk_cdclk_ratio(i915, >> cdclk_config), >> +cdclk_config->joined_mbus); >> +} >> + >> static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private >> *i915, >> const struct >> intel_cdclk_config *old_cdclk_config, >> const struct >> intel_cdclk_config *new_cdclk_config, >> @@ -2089,6 +2100,9 @@ static void bxt_set_cdclk(struct drm_i915_private >> *dev_priv, >> return; >> } >> >> +if (DISPLAY_VER(dev_priv) >= 20 && cdclk < >> dev_priv->display.cdclk.hw.cdclk) >> +xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config); >> + >> if (cdclk_compute_crawl_and_squash_midpoint(dev_priv, >> _priv->display.cdclk.hw, >> cdclk_config, >> _cdclk_config)) { >> _bxt_set_cdclk(dev_priv, _cdclk_config, pipe); >> @@ -2097,6 +2111,9 @@ static void bxt_set_cdclk(struct drm_i915_private >> *dev_priv, >> _bxt_set_cdclk(dev_priv, cdclk_config, pipe); >> } >> >> +if (DISPLAY_VER(dev_priv) >= 20 && cdclk > >> dev_priv->display.cdclk.hw.cdclk) >> +xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config); >> + >> if (DISPLAY_VER(dev_priv) >= 14) >> /* >> * NOOP - No Pcode communication needed for >> @@ -3179,6 +3196,20 @@ int intel_cdclk_atomic_check(struct >> intel_atomic_state *state,
Re: [PATCH 6/8] drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes
On Mon, Mar 04, 2024 at 03:30:25PM -0300, Gustavo Sousa wrote: > Commit 394b4b7df9f7 ("drm/i915/lnl: Add CDCLK table") and commit > 3d3696c0fed1 ("drm/i915/lnl: Start using CDCLK through PLL") started > adding support for CDCLK programming support for Xe2LPD. One final piece > is missing, which is the programming necessary for changed in the ratio > between MDCLK and CDCLK. Let's do that now. > > BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the > ratio between MDCLK and CDCLK changes. The updates must be done before > changing the CDCLK when decreasing the frequency; or after it when > increasing the frequency. > > Ratio-related updates to MBUS_CTL also depend on the state of MBus > joining, so they are performed by either CDCLK change sequence or by > changes in MBus joining. Since one might happen independently of the > other, we need to make sure that both logics see the necessary state > values when programming that register. MBus joining logic needs to know > the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio > field of struct intel_dbuf_state. > > For the CDCLK logic, we need to have something similar: we need to > propagate the status of MBus joining to struct intel_cdclk_state. Do > that by adding the field joined_mbus to struct intel_cdclk_config. > (Preferably, that field would be added to intel_cdclk_state, however > currently only intel_cdclk_config is passed down to the functions that > do the register programming. We might revisit this decision if we find > that refactoring the code to pass the whole intel_cdclk_state is worth > it.) > > Bspec: 68864, 68868, 69090, 69482 > Signed-off-by: Gustavo Sousa > --- > drivers/gpu/drm/i915/display/intel_cdclk.c| 31 ++ > drivers/gpu/drm/i915/display/intel_cdclk.h| 3 ++ > drivers/gpu/drm/i915/display/skl_watermark.c | 40 +++ > drivers/gpu/drm/i915/display/skl_watermark.h | 1 + > .../gpu/drm/i915/display/skl_watermark_regs.h | 18 + > 5 files changed, 77 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 04a6e9806254..12753589072d 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -40,6 +40,7 @@ > #include "intel_psr.h" > #include "intel_vdsc.h" > #include "skl_watermark.h" > +#include "skl_watermark_regs.h" > #include "vlv_sideband.h" > > /** > @@ -1683,6 +1684,8 @@ static void bxt_get_cdclk(struct drm_i915_private > *dev_priv, > } > > out: > + if (DISPLAY_VER(dev_priv) >= 20) > + cdclk_config->joined_mbus = intel_de_read(dev_priv, MBUS_CTL) & > MBUS_JOIN; > /* >* Can't read this out :( Let's assume it's >* at least what the CDCLK frequency requires. > @@ -1908,6 +1911,14 @@ u8 intel_mdclk_cdclk_ratio(struct drm_i915_private > *i915, > } > } > > +static void xe2lpd_mdclk_cdclk_ratio_program(struct drm_i915_private *i915, > + const struct intel_cdclk_config > *cdclk_config) > +{ > + intel_dbuf_mdclk_cdclk_ratio_update(i915, > + intel_mdclk_cdclk_ratio(i915, > cdclk_config), > + cdclk_config->joined_mbus); > +} > + > static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private > *i915, > const struct > intel_cdclk_config *old_cdclk_config, > const struct > intel_cdclk_config *new_cdclk_config, > @@ -2089,6 +2100,9 @@ static void bxt_set_cdclk(struct drm_i915_private > *dev_priv, > return; > } > > + if (DISPLAY_VER(dev_priv) >= 20 && cdclk < > dev_priv->display.cdclk.hw.cdclk) > + xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config); > + > if (cdclk_compute_crawl_and_squash_midpoint(dev_priv, > _priv->display.cdclk.hw, > cdclk_config, > _cdclk_config)) { > _bxt_set_cdclk(dev_priv, _cdclk_config, pipe); > @@ -2097,6 +2111,9 @@ static void bxt_set_cdclk(struct drm_i915_private > *dev_priv, > _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > } > > + if (DISPLAY_VER(dev_priv) >= 20 && cdclk > > dev_priv->display.cdclk.hw.cdclk) > + xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config); > + > if (DISPLAY_VER(dev_priv) >= 14) > /* >* NOOP - No Pcode communication needed for > @@ -3179,6 +3196,20 @@ int intel_cdclk_atomic_check(struct intel_atomic_state > *state, > return 0; > } > > +int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool > joined_mbus) > +{ > + struct intel_cdclk_state *cdclk_state; > + > + cdclk_state = intel_atomic_get_cdclk_state(state); > + if
[PATCH 6/8] drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes
Commit 394b4b7df9f7 ("drm/i915/lnl: Add CDCLK table") and commit 3d3696c0fed1 ("drm/i915/lnl: Start using CDCLK through PLL") started adding support for CDCLK programming support for Xe2LPD. One final piece is missing, which is the programming necessary for changed in the ratio between MDCLK and CDCLK. Let's do that now. BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the ratio between MDCLK and CDCLK changes. The updates must be done before changing the CDCLK when decreasing the frequency; or after it when increasing the frequency. Ratio-related updates to MBUS_CTL also depend on the state of MBus joining, so they are performed by either CDCLK change sequence or by changes in MBus joining. Since one might happen independently of the other, we need to make sure that both logics see the necessary state values when programming that register. MBus joining logic needs to know the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio field of struct intel_dbuf_state. For the CDCLK logic, we need to have something similar: we need to propagate the status of MBus joining to struct intel_cdclk_state. Do that by adding the field joined_mbus to struct intel_cdclk_config. (Preferably, that field would be added to intel_cdclk_state, however currently only intel_cdclk_config is passed down to the functions that do the register programming. We might revisit this decision if we find that refactoring the code to pass the whole intel_cdclk_state is worth it.) Bspec: 68864, 68868, 69090, 69482 Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/i915/display/intel_cdclk.c| 31 ++ drivers/gpu/drm/i915/display/intel_cdclk.h| 3 ++ drivers/gpu/drm/i915/display/skl_watermark.c | 40 +++ drivers/gpu/drm/i915/display/skl_watermark.h | 1 + .../gpu/drm/i915/display/skl_watermark_regs.h | 18 + 5 files changed, 77 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 04a6e9806254..12753589072d 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -40,6 +40,7 @@ #include "intel_psr.h" #include "intel_vdsc.h" #include "skl_watermark.h" +#include "skl_watermark_regs.h" #include "vlv_sideband.h" /** @@ -1683,6 +1684,8 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv, } out: + if (DISPLAY_VER(dev_priv) >= 20) + cdclk_config->joined_mbus = intel_de_read(dev_priv, MBUS_CTL) & MBUS_JOIN; /* * Can't read this out :( Let's assume it's * at least what the CDCLK frequency requires. @@ -1908,6 +1911,14 @@ u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, } } +static void xe2lpd_mdclk_cdclk_ratio_program(struct drm_i915_private *i915, +const struct intel_cdclk_config *cdclk_config) +{ + intel_dbuf_mdclk_cdclk_ratio_update(i915, + intel_mdclk_cdclk_ratio(i915, cdclk_config), + cdclk_config->joined_mbus); +} + static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915, const struct intel_cdclk_config *old_cdclk_config, const struct intel_cdclk_config *new_cdclk_config, @@ -2089,6 +2100,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, return; } + if (DISPLAY_VER(dev_priv) >= 20 && cdclk < dev_priv->display.cdclk.hw.cdclk) + xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config); + if (cdclk_compute_crawl_and_squash_midpoint(dev_priv, _priv->display.cdclk.hw, cdclk_config, _cdclk_config)) { _bxt_set_cdclk(dev_priv, _cdclk_config, pipe); @@ -2097,6 +2111,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, _bxt_set_cdclk(dev_priv, cdclk_config, pipe); } + if (DISPLAY_VER(dev_priv) >= 20 && cdclk > dev_priv->display.cdclk.hw.cdclk) + xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config); + if (DISPLAY_VER(dev_priv) >= 14) /* * NOOP - No Pcode communication needed for @@ -3179,6 +3196,20 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state, return 0; } +int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus) +{ + struct intel_cdclk_state *cdclk_state; + + cdclk_state = intel_atomic_get_cdclk_state(state); + if (IS_ERR(cdclk_state)) + return PTR_ERR(cdclk_state); + + cdclk_state->actual.joined_mbus = joined_mbus; + cdclk_state->logical.joined_mbus = joined_mbus; + + return intel_atomic_lock_global_state(_state->base);