[PATCH v3 1/2] drm/i915/xe3lpd: Power request asserting/deasserting

[Kahola, Mika]

> -----Original Message-----
> From: Jadav, Raag <raag.jadav at intel.com>
> Sent: Thursday, 31 October 2024 14.55
> To: Kahola, Mika <mika.kahola at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Sousa, Gustavo <gustavo.sousa at intel.com>;
> jani.nikula at linux.intel.com
> 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 at intel.com>
> > ---
> >  .../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