| Message ID | 20241021222744.294371-8-gustavo.sousa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD | expand |
On Mon, 21 Oct 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote: > There are extra registers that require the DMC wakelock when specific > dynamic DC states are in place. Add the table ranges for them and use > the correct table depending on the allowed DC states. > > Bspec: 71583 > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- > 1 file changed, 108 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c > index d597cc825f64..8bf2f32be859 100644 > --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c > +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c > @@ -5,6 +5,7 @@ > > #include <linux/kernel.h> > > +#include "i915_reg.h" > #include "intel_de.h" > #include "intel_dmc.h" > #include "intel_dmc_regs.h" > @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { > {}, > }; > > +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { > + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ > + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ > + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ > + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ > + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ > + > + /* DBUF_CTL_* */ > + { .start = 0x44300, .end = 0x44300 }, > + { .start = 0x44304, .end = 0x44304 }, > + { .start = 0x44f00, .end = 0x44f00 }, > + { .start = 0x44f04, .end = 0x44f04 }, > + { .start = 0x44fe8, .end = 0x44fe8 }, > + { .start = 0x45008, .end = 0x45008 }, > + > + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ > + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ > + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ > + > + /* TRANS_CMTG_CTL_* */ > + { .start = 0x6fa88, .end = 0x6fa88 }, > + { .start = 0x6fb88, .end = 0x6fb88 }, > + > + { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ > + { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ > + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ > + { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ > + { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ > + { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ > + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ > + > + {}, > +}; > + > +static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { > + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ > + > + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ > + > + /* DBUF_CTL_* */ > + { .start = 0x44300, .end = 0x44300 }, > + { .start = 0x44304, .end = 0x44304 }, > + { .start = 0x44f00, .end = 0x44f00 }, > + { .start = 0x44f04, .end = 0x44f04 }, > + { .start = 0x44fe8, .end = 0x44fe8 }, > + { .start = 0x45008, .end = 0x45008 }, > + > + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ > + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ > + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ > + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ > + > + /* Scanline registers */ > + { .start = 0x70000, .end = 0x70000 }, > + { .start = 0x70004, .end = 0x70004 }, > + { .start = 0x70014, .end = 0x70014 }, > + { .start = 0x70018, .end = 0x70018 }, > + { .start = 0x71000, .end = 0x71000 }, > + { .start = 0x71004, .end = 0x71004 }, > + { .start = 0x71014, .end = 0x71014 }, > + { .start = 0x71018, .end = 0x71018 }, > + { .start = 0x72000, .end = 0x72000 }, > + { .start = 0x72004, .end = 0x72004 }, > + { .start = 0x72014, .end = 0x72014 }, > + { .start = 0x72018, .end = 0x72018 }, > + { .start = 0x73000, .end = 0x73000 }, > + { .start = 0x73004, .end = 0x73004 }, > + { .start = 0x73014, .end = 0x73014 }, > + { .start = 0x73018, .end = 0x73018 }, > + { .start = 0x7b000, .end = 0x7b000 }, > + { .start = 0x7b004, .end = 0x7b004 }, > + { .start = 0x7b014, .end = 0x7b014 }, > + { .start = 0x7b018, .end = 0x7b018 }, > + { .start = 0x7c000, .end = 0x7c000 }, > + { .start = 0x7c004, .end = 0x7c004 }, > + { .start = 0x7c014, .end = 0x7c014 }, > + { .start = 0x7c018, .end = 0x7c018 }, > + > + {}, > +}; > + > static void __intel_dmc_wl_release(struct intel_display *display) > { > struct drm_i915_private *i915 = to_i915(display->drm); > @@ -106,9 +188,31 @@ static bool intel_dmc_wl_addr_in_range(u32 address, > return false; > } > > -static bool intel_dmc_wl_check_range(u32 address) > +static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address) > { > - return intel_dmc_wl_addr_in_range(address, lnl_wl_range); > + const struct intel_dmc_wl_range *ranges; > + > + ranges = lnl_wl_range; > + > + if (intel_dmc_wl_addr_in_range(address, ranges)) > + return true; > + > + switch (display->power.domains.dc_state) { This file has no business looking at power domain guts. Use or add interfaces instead of poking at data directly. > + case DC_STATE_EN_DC3CO: > + ranges = xe3lpd_dc3co_wl_ranges; > + break; > + case DC_STATE_EN_UPTO_DC5: > + case DC_STATE_EN_UPTO_DC6: > + ranges = xe3lpd_dc5_dc6_wl_ranges; > + break; > + default: > + ranges = NULL; > + } > + > + if (ranges && intel_dmc_wl_addr_in_range(address, ranges)) > + return true; > + > + return false; > } > > static bool __intel_dmc_wl_supported(struct intel_display *display) > @@ -195,7 +299,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg) > if (!__intel_dmc_wl_supported(display)) > return; > > - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) > + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) Side note, unrelated to this patch, i915_reg_t is supposed to be opaque, nobody should look at reg.reg directly, there's i915_mmio_reg_offset() for it. > return; > > spin_lock_irqsave(&wl->lock, flags); > @@ -247,7 +351,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg) > if (!__intel_dmc_wl_supported(display)) > return; > > - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) > + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) > return; > > spin_lock_irqsave(&wl->lock, flags);
On Mon, 21 Oct 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote: > There are extra registers that require the DMC wakelock when specific > dynamic DC states are in place. Add the table ranges for them and use > the correct table depending on the allowed DC states. > > Bspec: 71583 > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- > 1 file changed, 108 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c > index d597cc825f64..8bf2f32be859 100644 > --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c > +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c > @@ -5,6 +5,7 @@ > > #include <linux/kernel.h> > > +#include "i915_reg.h" I think you mean i915_reg_defs.h > #include "intel_de.h" > #include "intel_dmc.h" > #include "intel_dmc_regs.h" > @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { > {}, > }; > > +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { > + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ > + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ > + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ > + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ > + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ > + > + /* DBUF_CTL_* */ > + { .start = 0x44300, .end = 0x44300 }, > + { .start = 0x44304, .end = 0x44304 }, > + { .start = 0x44f00, .end = 0x44f00 }, > + { .start = 0x44f04, .end = 0x44f04 }, > + { .start = 0x44fe8, .end = 0x44fe8 }, > + { .start = 0x45008, .end = 0x45008 }, > + > + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ > + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ > + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ > + > + /* TRANS_CMTG_CTL_* */ > + { .start = 0x6fa88, .end = 0x6fa88 }, > + { .start = 0x6fb88, .end = 0x6fb88 }, > + > + { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ > + { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ > + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ > + { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ > + { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ > + { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ > + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ > + > + {}, > +}; > + > +static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { > + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ > + > + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ > + > + /* DBUF_CTL_* */ > + { .start = 0x44300, .end = 0x44300 }, > + { .start = 0x44304, .end = 0x44304 }, > + { .start = 0x44f00, .end = 0x44f00 }, > + { .start = 0x44f04, .end = 0x44f04 }, > + { .start = 0x44fe8, .end = 0x44fe8 }, > + { .start = 0x45008, .end = 0x45008 }, > + > + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ > + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ > + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ > + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ > + > + /* Scanline registers */ > + { .start = 0x70000, .end = 0x70000 }, > + { .start = 0x70004, .end = 0x70004 }, > + { .start = 0x70014, .end = 0x70014 }, > + { .start = 0x70018, .end = 0x70018 }, > + { .start = 0x71000, .end = 0x71000 }, > + { .start = 0x71004, .end = 0x71004 }, > + { .start = 0x71014, .end = 0x71014 }, > + { .start = 0x71018, .end = 0x71018 }, > + { .start = 0x72000, .end = 0x72000 }, > + { .start = 0x72004, .end = 0x72004 }, > + { .start = 0x72014, .end = 0x72014 }, > + { .start = 0x72018, .end = 0x72018 }, > + { .start = 0x73000, .end = 0x73000 }, > + { .start = 0x73004, .end = 0x73004 }, > + { .start = 0x73014, .end = 0x73014 }, > + { .start = 0x73018, .end = 0x73018 }, > + { .start = 0x7b000, .end = 0x7b000 }, > + { .start = 0x7b004, .end = 0x7b004 }, > + { .start = 0x7b014, .end = 0x7b014 }, > + { .start = 0x7b018, .end = 0x7b018 }, > + { .start = 0x7c000, .end = 0x7c000 }, > + { .start = 0x7c004, .end = 0x7c004 }, > + { .start = 0x7c014, .end = 0x7c014 }, > + { .start = 0x7c018, .end = 0x7c018 }, > + > + {}, > +}; > + > static void __intel_dmc_wl_release(struct intel_display *display) > { > struct drm_i915_private *i915 = to_i915(display->drm); > @@ -106,9 +188,31 @@ static bool intel_dmc_wl_addr_in_range(u32 address, > return false; > } > > -static bool intel_dmc_wl_check_range(u32 address) > +static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address) > { > - return intel_dmc_wl_addr_in_range(address, lnl_wl_range); > + const struct intel_dmc_wl_range *ranges; > + > + ranges = lnl_wl_range; > + > + if (intel_dmc_wl_addr_in_range(address, ranges)) > + return true; > + > + switch (display->power.domains.dc_state) { > + case DC_STATE_EN_DC3CO: > + ranges = xe3lpd_dc3co_wl_ranges; > + break; > + case DC_STATE_EN_UPTO_DC5: > + case DC_STATE_EN_UPTO_DC6: > + ranges = xe3lpd_dc5_dc6_wl_ranges; > + break; > + default: > + ranges = NULL; > + } > + > + if (ranges && intel_dmc_wl_addr_in_range(address, ranges)) > + return true; > + > + return false; > } > > static bool __intel_dmc_wl_supported(struct intel_display *display) > @@ -195,7 +299,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg) > if (!__intel_dmc_wl_supported(display)) > return; > > - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) > + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) > return; > > spin_lock_irqsave(&wl->lock, flags); > @@ -247,7 +351,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg) > if (!__intel_dmc_wl_supported(display)) > return; > > - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) > + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) > return; > > spin_lock_irqsave(&wl->lock, flags);
Quoting Jani Nikula (2024-10-22 05:03:21-03:00) >On Mon, 21 Oct 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote: >> There are extra registers that require the DMC wakelock when specific >> dynamic DC states are in place. Add the table ranges for them and use >> the correct table depending on the allowed DC states. >> >> Bspec: 71583 >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- >> 1 file changed, 108 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> index d597cc825f64..8bf2f32be859 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> @@ -5,6 +5,7 @@ >> >> #include <linux/kernel.h> >> >> +#include "i915_reg.h" >> #include "intel_de.h" >> #include "intel_dmc.h" >> #include "intel_dmc_regs.h" >> @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { >> {}, >> }; >> >> +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { >> + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ >> + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ >> + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ >> + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ >> + >> + /* DBUF_CTL_* */ >> + { .start = 0x44300, .end = 0x44300 }, >> + { .start = 0x44304, .end = 0x44304 }, >> + { .start = 0x44f00, .end = 0x44f00 }, >> + { .start = 0x44f04, .end = 0x44f04 }, >> + { .start = 0x44fe8, .end = 0x44fe8 }, >> + { .start = 0x45008, .end = 0x45008 }, >> + >> + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> + >> + /* TRANS_CMTG_CTL_* */ >> + { .start = 0x6fa88, .end = 0x6fa88 }, >> + { .start = 0x6fb88, .end = 0x6fb88 }, >> + >> + { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ >> + { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ >> + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >> + { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ >> + { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ >> + { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ >> + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >> + >> + {}, >> +}; >> + >> +static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { >> + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >> + >> + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> + >> + /* DBUF_CTL_* */ >> + { .start = 0x44300, .end = 0x44300 }, >> + { .start = 0x44304, .end = 0x44304 }, >> + { .start = 0x44f00, .end = 0x44f00 }, >> + { .start = 0x44f04, .end = 0x44f04 }, >> + { .start = 0x44fe8, .end = 0x44fe8 }, >> + { .start = 0x45008, .end = 0x45008 }, >> + >> + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >> + >> + /* Scanline registers */ >> + { .start = 0x70000, .end = 0x70000 }, >> + { .start = 0x70004, .end = 0x70004 }, >> + { .start = 0x70014, .end = 0x70014 }, >> + { .start = 0x70018, .end = 0x70018 }, >> + { .start = 0x71000, .end = 0x71000 }, >> + { .start = 0x71004, .end = 0x71004 }, >> + { .start = 0x71014, .end = 0x71014 }, >> + { .start = 0x71018, .end = 0x71018 }, >> + { .start = 0x72000, .end = 0x72000 }, >> + { .start = 0x72004, .end = 0x72004 }, >> + { .start = 0x72014, .end = 0x72014 }, >> + { .start = 0x72018, .end = 0x72018 }, >> + { .start = 0x73000, .end = 0x73000 }, >> + { .start = 0x73004, .end = 0x73004 }, >> + { .start = 0x73014, .end = 0x73014 }, >> + { .start = 0x73018, .end = 0x73018 }, >> + { .start = 0x7b000, .end = 0x7b000 }, >> + { .start = 0x7b004, .end = 0x7b004 }, >> + { .start = 0x7b014, .end = 0x7b014 }, >> + { .start = 0x7b018, .end = 0x7b018 }, >> + { .start = 0x7c000, .end = 0x7c000 }, >> + { .start = 0x7c004, .end = 0x7c004 }, >> + { .start = 0x7c014, .end = 0x7c014 }, >> + { .start = 0x7c018, .end = 0x7c018 }, >> + >> + {}, >> +}; >> + >> static void __intel_dmc_wl_release(struct intel_display *display) >> { >> struct drm_i915_private *i915 = to_i915(display->drm); >> @@ -106,9 +188,31 @@ static bool intel_dmc_wl_addr_in_range(u32 address, >> return false; >> } >> >> -static bool intel_dmc_wl_check_range(u32 address) >> +static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address) >> { >> - return intel_dmc_wl_addr_in_range(address, lnl_wl_range); >> + const struct intel_dmc_wl_range *ranges; >> + >> + ranges = lnl_wl_range; >> + >> + if (intel_dmc_wl_addr_in_range(address, ranges)) >> + return true; >> + >> + switch (display->power.domains.dc_state) { > >This file has no business looking at power domain guts. Use or add >interfaces instead of poking at data directly. Good point. Thanks. > >> + case DC_STATE_EN_DC3CO: >> + ranges = xe3lpd_dc3co_wl_ranges; >> + break; >> + case DC_STATE_EN_UPTO_DC5: >> + case DC_STATE_EN_UPTO_DC6: >> + ranges = xe3lpd_dc5_dc6_wl_ranges; >> + break; >> + default: >> + ranges = NULL; >> + } >> + >> + if (ranges && intel_dmc_wl_addr_in_range(address, ranges)) >> + return true; >> + >> + return false; >> } >> >> static bool __intel_dmc_wl_supported(struct intel_display *display) >> @@ -195,7 +299,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg) >> if (!__intel_dmc_wl_supported(display)) >> return; >> >> - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) >> + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) > >Side note, unrelated to this patch, i915_reg_t is supposed to be opaque, >nobody should look at reg.reg directly, there's i915_mmio_reg_offset() >for it. Ah, cool. I'll add a patch in v2 to have the whole file using i915_mmio_reg_offset(). Thanks. -- Gustavo Sousa > >> return; >> >> spin_lock_irqsave(&wl->lock, flags); >> @@ -247,7 +351,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg) >> if (!__intel_dmc_wl_supported(display)) >> return; >> >> - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) >> + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) >> return; >> >> spin_lock_irqsave(&wl->lock, flags); > >-- >Jani Nikula, Intel
Quoting Jani Nikula (2024-10-22 05:03:50-03:00) >On Mon, 21 Oct 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote: >> There are extra registers that require the DMC wakelock when specific >> dynamic DC states are in place. Add the table ranges for them and use >> the correct table depending on the allowed DC states. >> >> Bspec: 71583 >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- >> 1 file changed, 108 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> index d597cc825f64..8bf2f32be859 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> @@ -5,6 +5,7 @@ >> >> #include <linux/kernel.h> >> >> +#include "i915_reg.h" > >I think you mean i915_reg_defs.h Not really. I included i915_reg.h because of the DC_STATE_EN_* defines, which are currently being defined there. -- Gustavo Sousa > >> #include "intel_de.h" >> #include "intel_dmc.h" >> #include "intel_dmc_regs.h" >> @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { >> {}, >> }; >> >> +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { >> + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ >> + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ >> + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ >> + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ >> + >> + /* DBUF_CTL_* */ >> + { .start = 0x44300, .end = 0x44300 }, >> + { .start = 0x44304, .end = 0x44304 }, >> + { .start = 0x44f00, .end = 0x44f00 }, >> + { .start = 0x44f04, .end = 0x44f04 }, >> + { .start = 0x44fe8, .end = 0x44fe8 }, >> + { .start = 0x45008, .end = 0x45008 }, >> + >> + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> + >> + /* TRANS_CMTG_CTL_* */ >> + { .start = 0x6fa88, .end = 0x6fa88 }, >> + { .start = 0x6fb88, .end = 0x6fb88 }, >> + >> + { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ >> + { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ >> + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >> + { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ >> + { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ >> + { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ >> + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >> + >> + {}, >> +}; >> + >> +static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { >> + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >> + >> + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> + >> + /* DBUF_CTL_* */ >> + { .start = 0x44300, .end = 0x44300 }, >> + { .start = 0x44304, .end = 0x44304 }, >> + { .start = 0x44f00, .end = 0x44f00 }, >> + { .start = 0x44f04, .end = 0x44f04 }, >> + { .start = 0x44fe8, .end = 0x44fe8 }, >> + { .start = 0x45008, .end = 0x45008 }, >> + >> + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >> + >> + /* Scanline registers */ >> + { .start = 0x70000, .end = 0x70000 }, >> + { .start = 0x70004, .end = 0x70004 }, >> + { .start = 0x70014, .end = 0x70014 }, >> + { .start = 0x70018, .end = 0x70018 }, >> + { .start = 0x71000, .end = 0x71000 }, >> + { .start = 0x71004, .end = 0x71004 }, >> + { .start = 0x71014, .end = 0x71014 }, >> + { .start = 0x71018, .end = 0x71018 }, >> + { .start = 0x72000, .end = 0x72000 }, >> + { .start = 0x72004, .end = 0x72004 }, >> + { .start = 0x72014, .end = 0x72014 }, >> + { .start = 0x72018, .end = 0x72018 }, >> + { .start = 0x73000, .end = 0x73000 }, >> + { .start = 0x73004, .end = 0x73004 }, >> + { .start = 0x73014, .end = 0x73014 }, >> + { .start = 0x73018, .end = 0x73018 }, >> + { .start = 0x7b000, .end = 0x7b000 }, >> + { .start = 0x7b004, .end = 0x7b004 }, >> + { .start = 0x7b014, .end = 0x7b014 }, >> + { .start = 0x7b018, .end = 0x7b018 }, >> + { .start = 0x7c000, .end = 0x7c000 }, >> + { .start = 0x7c004, .end = 0x7c004 }, >> + { .start = 0x7c014, .end = 0x7c014 }, >> + { .start = 0x7c018, .end = 0x7c018 }, >> + >> + {}, >> +}; >> + >> static void __intel_dmc_wl_release(struct intel_display *display) >> { >> struct drm_i915_private *i915 = to_i915(display->drm); >> @@ -106,9 +188,31 @@ static bool intel_dmc_wl_addr_in_range(u32 address, >> return false; >> } >> >> -static bool intel_dmc_wl_check_range(u32 address) >> +static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address) >> { >> - return intel_dmc_wl_addr_in_range(address, lnl_wl_range); >> + const struct intel_dmc_wl_range *ranges; >> + >> + ranges = lnl_wl_range; >> + >> + if (intel_dmc_wl_addr_in_range(address, ranges)) >> + return true; >> + >> + switch (display->power.domains.dc_state) { >> + case DC_STATE_EN_DC3CO: >> + ranges = xe3lpd_dc3co_wl_ranges; >> + break; >> + case DC_STATE_EN_UPTO_DC5: >> + case DC_STATE_EN_UPTO_DC6: >> + ranges = xe3lpd_dc5_dc6_wl_ranges; >> + break; >> + default: >> + ranges = NULL; >> + } >> + >> + if (ranges && intel_dmc_wl_addr_in_range(address, ranges)) >> + return true; >> + >> + return false; >> } >> >> static bool __intel_dmc_wl_supported(struct intel_display *display) >> @@ -195,7 +299,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg) >> if (!__intel_dmc_wl_supported(display)) >> return; >> >> - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) >> + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) >> return; >> >> spin_lock_irqsave(&wl->lock, flags); >> @@ -247,7 +351,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg) >> if (!__intel_dmc_wl_supported(display)) >> return; >> >> - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) >> + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) >> return; >> >> spin_lock_irqsave(&wl->lock, flags); > >-- >Jani Nikula, Intel
Quoting Gustavo Sousa (2024-10-21 19:27:26-03:00) >There are extra registers that require the DMC wakelock when specific >dynamic DC states are in place. Add the table ranges for them and use >the correct table depending on the allowed DC states. > >Bspec: 71583 For anyone who is reviewing this and doesn't find the ranges in that Bspec page: I forgot to mention that the ranges are not yet listed there and that I got them directly from the display hardware team with a note that they would be updating the Bspec. -- Gustavo Sousa >Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> >--- > drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- > 1 file changed, 108 insertions(+), 4 deletions(-) > >diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >index d597cc825f64..8bf2f32be859 100644 >--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c >+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >@@ -5,6 +5,7 @@ > > #include <linux/kernel.h> > >+#include "i915_reg.h" > #include "intel_de.h" > #include "intel_dmc.h" > #include "intel_dmc_regs.h" >@@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { > {}, > }; > >+static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { >+ { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ >+ { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ >+ { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >+ { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ >+ { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ >+ >+ /* DBUF_CTL_* */ >+ { .start = 0x44300, .end = 0x44300 }, >+ { .start = 0x44304, .end = 0x44304 }, >+ { .start = 0x44f00, .end = 0x44f00 }, >+ { .start = 0x44f04, .end = 0x44f04 }, >+ { .start = 0x44fe8, .end = 0x44fe8 }, >+ { .start = 0x45008, .end = 0x45008 }, >+ >+ { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >+ { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >+ { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >+ >+ /* TRANS_CMTG_CTL_* */ >+ { .start = 0x6fa88, .end = 0x6fa88 }, >+ { .start = 0x6fb88, .end = 0x6fb88 }, >+ >+ { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ >+ { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ >+ { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >+ { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ >+ { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ >+ { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ >+ { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >+ >+ {}, >+}; >+ >+static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { >+ { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >+ >+ { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >+ >+ /* DBUF_CTL_* */ >+ { .start = 0x44300, .end = 0x44300 }, >+ { .start = 0x44304, .end = 0x44304 }, >+ { .start = 0x44f00, .end = 0x44f00 }, >+ { .start = 0x44f04, .end = 0x44f04 }, >+ { .start = 0x44fe8, .end = 0x44fe8 }, >+ { .start = 0x45008, .end = 0x45008 }, >+ >+ { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >+ { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >+ { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >+ { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >+ >+ /* Scanline registers */ >+ { .start = 0x70000, .end = 0x70000 }, >+ { .start = 0x70004, .end = 0x70004 }, >+ { .start = 0x70014, .end = 0x70014 }, >+ { .start = 0x70018, .end = 0x70018 }, >+ { .start = 0x71000, .end = 0x71000 }, >+ { .start = 0x71004, .end = 0x71004 }, >+ { .start = 0x71014, .end = 0x71014 }, >+ { .start = 0x71018, .end = 0x71018 }, >+ { .start = 0x72000, .end = 0x72000 }, >+ { .start = 0x72004, .end = 0x72004 }, >+ { .start = 0x72014, .end = 0x72014 }, >+ { .start = 0x72018, .end = 0x72018 }, >+ { .start = 0x73000, .end = 0x73000 }, >+ { .start = 0x73004, .end = 0x73004 }, >+ { .start = 0x73014, .end = 0x73014 }, >+ { .start = 0x73018, .end = 0x73018 }, >+ { .start = 0x7b000, .end = 0x7b000 }, >+ { .start = 0x7b004, .end = 0x7b004 }, >+ { .start = 0x7b014, .end = 0x7b014 }, >+ { .start = 0x7b018, .end = 0x7b018 }, >+ { .start = 0x7c000, .end = 0x7c000 }, >+ { .start = 0x7c004, .end = 0x7c004 }, >+ { .start = 0x7c014, .end = 0x7c014 }, >+ { .start = 0x7c018, .end = 0x7c018 }, >+ >+ {}, >+}; >+ > static void __intel_dmc_wl_release(struct intel_display *display) > { > struct drm_i915_private *i915 = to_i915(display->drm); >@@ -106,9 +188,31 @@ static bool intel_dmc_wl_addr_in_range(u32 address, > return false; > } > >-static bool intel_dmc_wl_check_range(u32 address) >+static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address) > { >- return intel_dmc_wl_addr_in_range(address, lnl_wl_range); >+ const struct intel_dmc_wl_range *ranges; >+ >+ ranges = lnl_wl_range; >+ >+ if (intel_dmc_wl_addr_in_range(address, ranges)) >+ return true; >+ >+ switch (display->power.domains.dc_state) { >+ case DC_STATE_EN_DC3CO: >+ ranges = xe3lpd_dc3co_wl_ranges; >+ break; >+ case DC_STATE_EN_UPTO_DC5: >+ case DC_STATE_EN_UPTO_DC6: >+ ranges = xe3lpd_dc5_dc6_wl_ranges; >+ break; >+ default: >+ ranges = NULL; >+ } >+ >+ if (ranges && intel_dmc_wl_addr_in_range(address, ranges)) >+ return true; >+ >+ return false; > } > > static bool __intel_dmc_wl_supported(struct intel_display *display) >@@ -195,7 +299,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg) > if (!__intel_dmc_wl_supported(display)) > return; > >- if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) >+ if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) > return; > > spin_lock_irqsave(&wl->lock, flags); >@@ -247,7 +351,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg) > if (!__intel_dmc_wl_supported(display)) > return; > >- if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) >+ if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) > return; > > spin_lock_irqsave(&wl->lock, flags); >-- >2.47.0 >
On Mon, 2024-10-21 at 19:27 -0300, Gustavo Sousa wrote: > There are extra registers that require the DMC wakelock when specific > dynamic DC states are in place. Add the table ranges for them and use > the correct table depending on the allowed DC states. > > Bspec: 71583 > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- > 1 file changed, 108 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c > index d597cc825f64..8bf2f32be859 100644 > --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c > +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c > @@ -5,6 +5,7 @@ > > #include <linux/kernel.h> > > +#include "i915_reg.h" > #include "intel_de.h" > #include "intel_dmc.h" > #include "intel_dmc_regs.h" > @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { > {}, > }; Do we still need the lnl_wl_range[]? This was sort of a place-holder with a very large range just for testing. I can see that there are at least some ranges in common between lnl_wl_range[] and the new range tables defined below. > +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { > + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ > + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ > + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ > + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ > + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ > + > + /* DBUF_CTL_* */ > + { .start = 0x44300, .end = 0x44300 }, > + { .start = 0x44304, .end = 0x44304 }, > + { .start = 0x44f00, .end = 0x44f00 }, > + { .start = 0x44f04, .end = 0x44f04 }, > + { .start = 0x44fe8, .end = 0x44fe8 }, > + { .start = 0x45008, .end = 0x45008 }, > + > + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ > + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ > + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ > + > + /* TRANS_CMTG_CTL_* */ > + { .start = 0x6fa88, .end = 0x6fa88 }, > + { .start = 0x6fb88, .end = 0x6fb88 }, These, for instance, are part of lnl_wl_range[]. > + > + { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ > + { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ > + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ > + { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ > + { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ > + { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ > + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ > + > + {}, > +}; > + > +static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { > + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ > + > + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ > + > + /* DBUF_CTL_* */ > + { .start = 0x44300, .end = 0x44300 }, > + { .start = 0x44304, .end = 0x44304 }, > + { .start = 0x44f00, .end = 0x44f00 }, > + { .start = 0x44f04, .end = 0x44f04 }, > + { .start = 0x44fe8, .end = 0x44fe8 }, > + { .start = 0x45008, .end = 0x45008 }, > + > + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ > + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ > + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ > + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ > + > + /* Scanline registers */ > + { .start = 0x70000, .end = 0x70000 }, > + { .start = 0x70004, .end = 0x70004 }, > + { .start = 0x70014, .end = 0x70014 }, > + { .start = 0x70018, .end = 0x70018 }, > + { .start = 0x71000, .end = 0x71000 }, > + { .start = 0x71004, .end = 0x71004 }, > + { .start = 0x71014, .end = 0x71014 }, > + { .start = 0x71018, .end = 0x71018 }, > + { .start = 0x72000, .end = 0x72000 }, > + { .start = 0x72004, .end = 0x72004 }, > + { .start = 0x72014, .end = 0x72014 }, > + { .start = 0x72018, .end = 0x72018 }, > + { .start = 0x73000, .end = 0x73000 }, > + { .start = 0x73004, .end = 0x73004 }, > + { .start = 0x73014, .end = 0x73014 }, > + { .start = 0x73018, .end = 0x73018 }, > + { .start = 0x7b000, .end = 0x7b000 }, > + { .start = 0x7b004, .end = 0x7b004 }, > + { .start = 0x7b014, .end = 0x7b014 }, > + { .start = 0x7b018, .end = 0x7b018 }, > + { .start = 0x7c000, .end = 0x7c000 }, > + { .start = 0x7c004, .end = 0x7c004 }, > + { .start = 0x7c014, .end = 0x7c014 }, > + { .start = 0x7c018, .end = 0x7c018 }, And so are all these. -- Cheers, Luca.
Quoting Luca Coelho (2024-11-01 09:51:48-03:00) >On Mon, 2024-10-21 at 19:27 -0300, Gustavo Sousa wrote: >> There are extra registers that require the DMC wakelock when specific >> dynamic DC states are in place. Add the table ranges for them and use >> the correct table depending on the allowed DC states. >> >> Bspec: 71583 >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- >> 1 file changed, 108 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> index d597cc825f64..8bf2f32be859 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> @@ -5,6 +5,7 @@ >> >> #include <linux/kernel.h> >> >> +#include "i915_reg.h" >> #include "intel_de.h" >> #include "intel_dmc.h" >> #include "intel_dmc_regs.h" >> @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { >> {}, >> }; > >Do we still need the lnl_wl_range[]? This was sort of a place-holder >with a very large range just for testing. I can see that there are at >least some ranges in common between lnl_wl_range[] and the new range >tables defined below. Yes, although we could do some homework to get a more accurate set of ranges. Now, about the different tables: - lnl_wl_range should be about ranges of registers that are powered down during DC states and that the HW requires DC exit for proper access. - xe3lpd_{dc5_dc6,dc3co}_wl_ranges are registers that are touched by the DMC and need the wakelock for properly restoring the correct value before accessing them. Maybe a comment in the code explaining the above is warranted? > > >> +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { >> + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ >> + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ >> + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ >> + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ >> + >> + /* DBUF_CTL_* */ >> + { .start = 0x44300, .end = 0x44300 }, >> + { .start = 0x44304, .end = 0x44304 }, >> + { .start = 0x44f00, .end = 0x44f00 }, >> + { .start = 0x44f04, .end = 0x44f04 }, >> + { .start = 0x44fe8, .end = 0x44fe8 }, >> + { .start = 0x45008, .end = 0x45008 }, >> + >> + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> + >> + /* TRANS_CMTG_CTL_* */ >> + { .start = 0x6fa88, .end = 0x6fa88 }, >> + { .start = 0x6fb88, .end = 0x6fb88 }, > >These, for instance, are part of lnl_wl_range[]. Given my clarification above about the different purposes of the ranges, I think we should stick to keeping the same values from the (soon to be?) documented tables, even if there is some small redundancy. Otherwise we would require the programmer to remember to check ranges in the "more general" table every time a DC state-specific one needs to be added or updated. -- Gustavo Sousa > > >> + >> + { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ >> + { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ >> + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >> + { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ >> + { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ >> + { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ >> + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >> + >> + {}, >> +}; >> + >> +static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { >> + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >> + >> + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> + >> + /* DBUF_CTL_* */ >> + { .start = 0x44300, .end = 0x44300 }, >> + { .start = 0x44304, .end = 0x44304 }, >> + { .start = 0x44f00, .end = 0x44f00 }, >> + { .start = 0x44f04, .end = 0x44f04 }, >> + { .start = 0x44fe8, .end = 0x44fe8 }, >> + { .start = 0x45008, .end = 0x45008 }, >> + >> + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >> + >> + /* Scanline registers */ >> + { .start = 0x70000, .end = 0x70000 }, >> + { .start = 0x70004, .end = 0x70004 }, >> + { .start = 0x70014, .end = 0x70014 }, >> + { .start = 0x70018, .end = 0x70018 }, >> + { .start = 0x71000, .end = 0x71000 }, >> + { .start = 0x71004, .end = 0x71004 }, >> + { .start = 0x71014, .end = 0x71014 }, >> + { .start = 0x71018, .end = 0x71018 }, >> + { .start = 0x72000, .end = 0x72000 }, >> + { .start = 0x72004, .end = 0x72004 }, >> + { .start = 0x72014, .end = 0x72014 }, >> + { .start = 0x72018, .end = 0x72018 }, >> + { .start = 0x73000, .end = 0x73000 }, >> + { .start = 0x73004, .end = 0x73004 }, >> + { .start = 0x73014, .end = 0x73014 }, >> + { .start = 0x73018, .end = 0x73018 }, >> + { .start = 0x7b000, .end = 0x7b000 }, >> + { .start = 0x7b004, .end = 0x7b004 }, >> + { .start = 0x7b014, .end = 0x7b014 }, >> + { .start = 0x7b018, .end = 0x7b018 }, >> + { .start = 0x7c000, .end = 0x7c000 }, >> + { .start = 0x7c004, .end = 0x7c004 }, >> + { .start = 0x7c014, .end = 0x7c014 }, >> + { .start = 0x7c018, .end = 0x7c018 }, > >And so are all these. > > >-- >Cheers, >Luca.
Quoting Jani Nikula (2024-10-22 05:03:21-03:00) >On Mon, 21 Oct 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote: >> There are extra registers that require the DMC wakelock when specific >> dynamic DC states are in place. Add the table ranges for them and use >> the correct table depending on the allowed DC states. >> >> Bspec: 71583 >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- >> 1 file changed, 108 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> index d597cc825f64..8bf2f32be859 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> @@ -5,6 +5,7 @@ >> >> #include <linux/kernel.h> >> >> +#include "i915_reg.h" >> #include "intel_de.h" >> #include "intel_dmc.h" >> #include "intel_dmc_regs.h" >> @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { >> {}, >> }; >> >> +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { >> + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ >> + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ >> + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ >> + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ >> + >> + /* DBUF_CTL_* */ >> + { .start = 0x44300, .end = 0x44300 }, >> + { .start = 0x44304, .end = 0x44304 }, >> + { .start = 0x44f00, .end = 0x44f00 }, >> + { .start = 0x44f04, .end = 0x44f04 }, >> + { .start = 0x44fe8, .end = 0x44fe8 }, >> + { .start = 0x45008, .end = 0x45008 }, >> + >> + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> + >> + /* TRANS_CMTG_CTL_* */ >> + { .start = 0x6fa88, .end = 0x6fa88 }, >> + { .start = 0x6fb88, .end = 0x6fb88 }, >> + >> + { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ >> + { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ >> + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >> + { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ >> + { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ >> + { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ >> + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >> + >> + {}, >> +}; >> + >> +static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { >> + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ >> + >> + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> + >> + /* DBUF_CTL_* */ >> + { .start = 0x44300, .end = 0x44300 }, >> + { .start = 0x44304, .end = 0x44304 }, >> + { .start = 0x44f00, .end = 0x44f00 }, >> + { .start = 0x44f04, .end = 0x44f04 }, >> + { .start = 0x44fe8, .end = 0x44fe8 }, >> + { .start = 0x45008, .end = 0x45008 }, >> + >> + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ >> + >> + /* Scanline registers */ >> + { .start = 0x70000, .end = 0x70000 }, >> + { .start = 0x70004, .end = 0x70004 }, >> + { .start = 0x70014, .end = 0x70014 }, >> + { .start = 0x70018, .end = 0x70018 }, >> + { .start = 0x71000, .end = 0x71000 }, >> + { .start = 0x71004, .end = 0x71004 }, >> + { .start = 0x71014, .end = 0x71014 }, >> + { .start = 0x71018, .end = 0x71018 }, >> + { .start = 0x72000, .end = 0x72000 }, >> + { .start = 0x72004, .end = 0x72004 }, >> + { .start = 0x72014, .end = 0x72014 }, >> + { .start = 0x72018, .end = 0x72018 }, >> + { .start = 0x73000, .end = 0x73000 }, >> + { .start = 0x73004, .end = 0x73004 }, >> + { .start = 0x73014, .end = 0x73014 }, >> + { .start = 0x73018, .end = 0x73018 }, >> + { .start = 0x7b000, .end = 0x7b000 }, >> + { .start = 0x7b004, .end = 0x7b004 }, >> + { .start = 0x7b014, .end = 0x7b014 }, >> + { .start = 0x7b018, .end = 0x7b018 }, >> + { .start = 0x7c000, .end = 0x7c000 }, >> + { .start = 0x7c004, .end = 0x7c004 }, >> + { .start = 0x7c014, .end = 0x7c014 }, >> + { .start = 0x7c018, .end = 0x7c018 }, >> + >> + {}, >> +}; >> + >> static void __intel_dmc_wl_release(struct intel_display *display) >> { >> struct drm_i915_private *i915 = to_i915(display->drm); >> @@ -106,9 +188,31 @@ static bool intel_dmc_wl_addr_in_range(u32 address, >> return false; >> } >> >> -static bool intel_dmc_wl_check_range(u32 address) >> +static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address) >> { >> - return intel_dmc_wl_addr_in_range(address, lnl_wl_range); >> + const struct intel_dmc_wl_range *ranges; >> + >> + ranges = lnl_wl_range; >> + >> + if (intel_dmc_wl_addr_in_range(address, ranges)) >> + return true; >> + >> + switch (display->power.domains.dc_state) { > >This file has no business looking at power domain guts. Use or add >interfaces instead of poking at data directly. I started adding a function intel_display_power_get_current_dc_state() here, but then realized that display->power.domains is protected by a mutex and we do not want to use it in an atomic context. So, in v2, to avoid rewriting the whole power domains code to use spinlocks, I decided to go with having a copy of dc_state struct intel_dmc_wl, which is set by intel_dmc_wl_enable(). -- Gustavo Sousa > >> + case DC_STATE_EN_DC3CO: >> + ranges = xe3lpd_dc3co_wl_ranges; >> + break; >> + case DC_STATE_EN_UPTO_DC5: >> + case DC_STATE_EN_UPTO_DC6: >> + ranges = xe3lpd_dc5_dc6_wl_ranges; >> + break; >> + default: >> + ranges = NULL; >> + } >> + >> + if (ranges && intel_dmc_wl_addr_in_range(address, ranges)) >> + return true; >> + >> + return false; >> } >> >> static bool __intel_dmc_wl_supported(struct intel_display *display) >> @@ -195,7 +299,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg) >> if (!__intel_dmc_wl_supported(display)) >> return; >> >> - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) >> + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) > >Side note, unrelated to this patch, i915_reg_t is supposed to be opaque, >nobody should look at reg.reg directly, there's i915_mmio_reg_offset() >for it. > >> return; >> >> spin_lock_irqsave(&wl->lock, flags); >> @@ -247,7 +351,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg) >> if (!__intel_dmc_wl_supported(display)) >> return; >> >> - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) >> + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) >> return; >> >> spin_lock_irqsave(&wl->lock, flags); > >-- >Jani Nikula, Intel
On Tue, 2024-11-05 at 10:00 -0300, Gustavo Sousa wrote: > Quoting Luca Coelho (2024-11-01 09:51:48-03:00) > > On Mon, 2024-10-21 at 19:27 -0300, Gustavo Sousa wrote: > > > There are extra registers that require the DMC wakelock when specific > > > dynamic DC states are in place. Add the table ranges for them and use > > > the correct table depending on the allowed DC states. > > > > > > Bspec: 71583 > > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- > > > 1 file changed, 108 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c > > > index d597cc825f64..8bf2f32be859 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c > > > @@ -5,6 +5,7 @@ > > > > > > #include <linux/kernel.h> > > > > > > +#include "i915_reg.h" > > > #include "intel_de.h" > > > #include "intel_dmc.h" > > > #include "intel_dmc_regs.h" > > > @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { > > > {}, > > > }; > > > > Do we still need the lnl_wl_range[]? This was sort of a place-holder > > with a very large range just for testing. I can see that there are at > > least some ranges in common between lnl_wl_range[] and the new range > > tables defined below. > > Yes, although we could do some homework to get a more accurate set of > ranges. > > Now, about the different tables: > > - lnl_wl_range should be about ranges of registers that are powered > down during DC states and that the HW requires DC exit for proper > access. > - xe3lpd_{dc5_dc6,dc3co}_wl_ranges are registers that are touched by > the DMC and need the wakelock for properly restoring the correct > value before accessing them. > > Maybe a comment in the code explaining the above is warranted? I think a better naming for the arrays is warranted. :) Wouldn't changing lnl_wl_range to base_wl_range or so be better? My point is that LNL is not related at all here (anymore). > > > +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { > > > + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ > > > + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ > > > + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ > > > + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ > > > + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ > > > + > > > + /* DBUF_CTL_* */ > > > + { .start = 0x44300, .end = 0x44300 }, > > > + { .start = 0x44304, .end = 0x44304 }, > > > + { .start = 0x44f00, .end = 0x44f00 }, > > > + { .start = 0x44f04, .end = 0x44f04 }, > > > + { .start = 0x44fe8, .end = 0x44fe8 }, > > > + { .start = 0x45008, .end = 0x45008 }, > > > + > > > + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ > > > + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ > > > + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ > > > + > > > + /* TRANS_CMTG_CTL_* */ > > > + { .start = 0x6fa88, .end = 0x6fa88 }, > > > + { .start = 0x6fb88, .end = 0x6fb88 }, > > > > These, for instance, are part of lnl_wl_range[]. > > Given my clarification above about the different purposes of the ranges, > I think we should stick to keeping the same values from the (soon to > be?) documented tables, even if there is some small redundancy. > Otherwise we would require the programmer to remember to check ranges in > the "more general" table every time a DC state-specific one needs to be > added or updated. Makes sense, I guess it's okay that the base table and the specialized tables are slightly redundant then. -- Cheers, Luca.
Quoting Luca Coelho (2024-11-06 08:47:07-03:00) >On Tue, 2024-11-05 at 10:00 -0300, Gustavo Sousa wrote: >> Quoting Luca Coelho (2024-11-01 09:51:48-03:00) >> > On Mon, 2024-10-21 at 19:27 -0300, Gustavo Sousa wrote: >> > > There are extra registers that require the DMC wakelock when specific >> > > dynamic DC states are in place. Add the table ranges for them and use >> > > the correct table depending on the allowed DC states. >> > > >> > > Bspec: 71583 >> > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> >> > > --- >> > > drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- >> > > 1 file changed, 108 insertions(+), 4 deletions(-) >> > > >> > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> > > index d597cc825f64..8bf2f32be859 100644 >> > > --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> > > +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c >> > > @@ -5,6 +5,7 @@ >> > > >> > > #include <linux/kernel.h> >> > > >> > > +#include "i915_reg.h" >> > > #include "intel_de.h" >> > > #include "intel_dmc.h" >> > > #include "intel_dmc_regs.h" >> > > @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { >> > > {}, >> > > }; >> > >> > Do we still need the lnl_wl_range[]? This was sort of a place-holder >> > with a very large range just for testing. I can see that there are at >> > least some ranges in common between lnl_wl_range[] and the new range >> > tables defined below. >> >> Yes, although we could do some homework to get a more accurate set of >> ranges. >> >> Now, about the different tables: >> >> - lnl_wl_range should be about ranges of registers that are powered >> down during DC states and that the HW requires DC exit for proper >> access. >> - xe3lpd_{dc5_dc6,dc3co}_wl_ranges are registers that are touched by >> the DMC and need the wakelock for properly restoring the correct >> value before accessing them. >> >> Maybe a comment in the code explaining the above is warranted? > >I think a better naming for the arrays is warranted. :) Wouldn't >changing lnl_wl_range to base_wl_range or so be better? My point is >that LNL is not related at all here (anymore). Yep, we could come up with better names for those variables. I went with: s/lnl_wl_range/powered_off_ranges/ s/xe3lpd_dc3co_wl_ranges/xe3lpd_dc3co_dmc_ranges/ s/xe3lpd_dc5_dc6_wl_ranges/xe3lpd_dc5_dc6_dmc_ranges/ And also added comments to differentiate their purposes. -- Gustavo Sousa > > >> > > +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { >> > > + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ >> > > + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ >> > > + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ >> > > + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ >> > > + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ >> > > + >> > > + /* DBUF_CTL_* */ >> > > + { .start = 0x44300, .end = 0x44300 }, >> > > + { .start = 0x44304, .end = 0x44304 }, >> > > + { .start = 0x44f00, .end = 0x44f00 }, >> > > + { .start = 0x44f04, .end = 0x44f04 }, >> > > + { .start = 0x44fe8, .end = 0x44fe8 }, >> > > + { .start = 0x45008, .end = 0x45008 }, >> > > + >> > > + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ >> > > + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ >> > > + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ >> > > + >> > > + /* TRANS_CMTG_CTL_* */ >> > > + { .start = 0x6fa88, .end = 0x6fa88 }, >> > > + { .start = 0x6fb88, .end = 0x6fb88 }, >> > >> > These, for instance, are part of lnl_wl_range[]. >> >> Given my clarification above about the different purposes of the ranges, >> I think we should stick to keeping the same values from the (soon to >> be?) documented tables, even if there is some small redundancy. >> Otherwise we would require the programmer to remember to check ranges in >> the "more general" table every time a DC state-specific one needs to be >> added or updated. > >Makes sense, I guess it's okay that the base table and the specialized >tables are slightly redundant then. > >-- >Cheers, >Luca.
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c index d597cc825f64..8bf2f32be859 100644 --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c @@ -5,6 +5,7 @@ #include <linux/kernel.h> +#include "i915_reg.h" #include "intel_de.h" #include "intel_dmc.h" #include "intel_dmc_regs.h" @@ -52,6 +53,87 @@ static struct intel_dmc_wl_range lnl_wl_range[] = { {}, }; +static struct intel_dmc_wl_range xe3lpd_dc5_dc6_wl_ranges[] = { + { .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */ + { .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */ + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ + { .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */ + { .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */ + + /* DBUF_CTL_* */ + { .start = 0x44300, .end = 0x44300 }, + { .start = 0x44304, .end = 0x44304 }, + { .start = 0x44f00, .end = 0x44f00 }, + { .start = 0x44f04, .end = 0x44f04 }, + { .start = 0x44fe8, .end = 0x44fe8 }, + { .start = 0x45008, .end = 0x45008 }, + + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ + + /* TRANS_CMTG_CTL_* */ + { .start = 0x6fa88, .end = 0x6fa88 }, + { .start = 0x6fb88, .end = 0x6fb88 }, + + { .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */ + { .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */ + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ + { .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */ + { .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */ + { .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */ + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ + + {}, +}; + +static struct intel_dmc_wl_range xe3lpd_dc3co_wl_ranges[] = { + { .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */ + + { .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */ + + /* DBUF_CTL_* */ + { .start = 0x44300, .end = 0x44300 }, + { .start = 0x44304, .end = 0x44304 }, + { .start = 0x44f00, .end = 0x44f00 }, + { .start = 0x44f04, .end = 0x44f04 }, + { .start = 0x44fe8, .end = 0x44fe8 }, + { .start = 0x45008, .end = 0x45008 }, + + { .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */ + { .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */ + { .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */ + { .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */ + + /* Scanline registers */ + { .start = 0x70000, .end = 0x70000 }, + { .start = 0x70004, .end = 0x70004 }, + { .start = 0x70014, .end = 0x70014 }, + { .start = 0x70018, .end = 0x70018 }, + { .start = 0x71000, .end = 0x71000 }, + { .start = 0x71004, .end = 0x71004 }, + { .start = 0x71014, .end = 0x71014 }, + { .start = 0x71018, .end = 0x71018 }, + { .start = 0x72000, .end = 0x72000 }, + { .start = 0x72004, .end = 0x72004 }, + { .start = 0x72014, .end = 0x72014 }, + { .start = 0x72018, .end = 0x72018 }, + { .start = 0x73000, .end = 0x73000 }, + { .start = 0x73004, .end = 0x73004 }, + { .start = 0x73014, .end = 0x73014 }, + { .start = 0x73018, .end = 0x73018 }, + { .start = 0x7b000, .end = 0x7b000 }, + { .start = 0x7b004, .end = 0x7b004 }, + { .start = 0x7b014, .end = 0x7b014 }, + { .start = 0x7b018, .end = 0x7b018 }, + { .start = 0x7c000, .end = 0x7c000 }, + { .start = 0x7c004, .end = 0x7c004 }, + { .start = 0x7c014, .end = 0x7c014 }, + { .start = 0x7c018, .end = 0x7c018 }, + + {}, +}; + static void __intel_dmc_wl_release(struct intel_display *display) { struct drm_i915_private *i915 = to_i915(display->drm); @@ -106,9 +188,31 @@ static bool intel_dmc_wl_addr_in_range(u32 address, return false; } -static bool intel_dmc_wl_check_range(u32 address) +static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address) { - return intel_dmc_wl_addr_in_range(address, lnl_wl_range); + const struct intel_dmc_wl_range *ranges; + + ranges = lnl_wl_range; + + if (intel_dmc_wl_addr_in_range(address, ranges)) + return true; + + switch (display->power.domains.dc_state) { + case DC_STATE_EN_DC3CO: + ranges = xe3lpd_dc3co_wl_ranges; + break; + case DC_STATE_EN_UPTO_DC5: + case DC_STATE_EN_UPTO_DC6: + ranges = xe3lpd_dc5_dc6_wl_ranges; + break; + default: + ranges = NULL; + } + + if (ranges && intel_dmc_wl_addr_in_range(address, ranges)) + return true; + + return false; } static bool __intel_dmc_wl_supported(struct intel_display *display) @@ -195,7 +299,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg) if (!__intel_dmc_wl_supported(display)) return; - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) return; spin_lock_irqsave(&wl->lock, flags); @@ -247,7 +351,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg) if (!__intel_dmc_wl_supported(display)) return; - if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg.reg)) + if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(display, reg.reg)) return; spin_lock_irqsave(&wl->lock, flags);
There are extra registers that require the DMC wakelock when specific dynamic DC states are in place. Add the table ranges for them and use the correct table depending on the allowed DC states. Bspec: 71583 Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> --- drivers/gpu/drm/i915/display/intel_dmc_wl.c | 112 +++++++++++++++++++- 1 file changed, 108 insertions(+), 4 deletions(-)