> -----Original Message----- > From: Jadav, Raag <raag.jadav@xxxxxxxxx> > Sent: Thursday, 31 October 2024 14.55 > To: Kahola, Mika <mika.kahola@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Sousa, Gustavo <gustavo.sousa@xxxxxxxxx>; > jani.nikula@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v3 1/2] drm/i915/xe3lpd: Power request > asserting/deasserting > > On Thu, Oct 31, 2024 at 01:40:26PM +0200, Mika Kahola wrote: > > There is a HW issue that arises when there are race conditions between > > TCSS entering/exiting TC7 or TC10 states while the driver is > > asserting/deasserting TCSS power request. As a workaround, Display > > driver will implement a mailbox sequence to ensure that the TCSS is in > > TC0 when TCSS power request is asserted/deasserted. > > > > The sequence is the following > > > > 1. Read mailbox command status and wait until run/busy bit is > > clear > > 2. Write mailbox data value '1' for power request asserting > > and '0' for power request deasserting 3. Write mailbox command > > run/busy bit and command value with 0x1 4. Read mailbox command and > > wait until run/busy bit is clear > > before continuing power request. > > > > v2: Rename WA function (Gustavo) > > Limit WA only for PTL platform with a TODO note (Gustavo) > > Add TCSS_DISP_MAILBOX_IN_CMD_RUN_BUSY for clarity when writing > > register data (Gustavo) > > Move register defs from i915_reg.h to intel_cx0_phy_regs.h > > (Gustavo) > > v3: use "struct intel_display" instead of "struct drm_i915_private" (Jani) > > Move defs above C10 definitions in the > > intel_cx0_phy_regs.h file (Gustavo) > > Move drm_WARN_ON() inside WA function (Gustavo) > > Rename workaround function as wa_14020908590() (Gustvo) > > Use boolean enable instead of if-else structure (Raag) > > > > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx> > > --- > > .../gpu/drm/i915/display/intel_cx0_phy_regs.h | 8 +++++ > > drivers/gpu/drm/i915/display/intel_tc.c | 32 +++++++++++++++++++ > > 2 files changed, 40 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > index f0e5c196eae4..5a0b55cca4a3 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > > @@ -200,6 +200,14 @@ > > #define XELPDP_SSC_ENABLE_PLLA REG_BIT(1) > > #define XELPDP_SSC_ENABLE_PLLB REG_BIT(0) > > > > +#define TCSS_DISP_MAILBOX_IN_CMD _MMIO(0x161300) > > +#define TCSS_DISP_MAILBOX_IN_CMD_RUN_BUSY REG_BIT(31) > > +#define TCSS_DISP_MAILBOX_IN_CMD_CMD_MASK REG_GENMASK(7, 0) > > +#define TCSS_DISP_MAILBOX_IN_CMD_DATA(val) > (TCSS_DISP_MAILBOX_IN_CMD_RUN_BUSY | \ > > + > REG_FIELD_PREP(TCSS_DISP_MAILBOX_IN_CMD_CMD_MASK, val)) > > + > > +#define TCSS_DISP_MAILBOX_IN_DATA _MMIO(0x161304) > > + > > /* C10 Vendor Registers */ > > #define PHY_C10_VDR_PLL(idx) (0xC00 + (idx)) > > #define C10_PLL0_FRACEN REG_BIT8(4) > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c > > b/drivers/gpu/drm/i915/display/intel_tc.c > > index b16c4d2d4077..dab2ac7ec0dd 100644 > > --- a/drivers/gpu/drm/i915/display/intel_tc.c > > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > > @@ -1013,6 +1013,31 @@ xelpdp_tc_phy_wait_for_tcss_power(struct > intel_tc_port *tc, bool enabled) > > return true; > > } > > > > +static void wa_14020908590(struct intel_display *display, > > + bool enable) > > Can be one line? Could be. This should fit within expected line length. > > Raag