On Thu, 2024-10-24 at 12:18 -0700, Matt Roper wrote: > On Thu, Oct 24, 2024 at 06:08:46AM +0000, Kahola, Mika wrote: > > > -----Original Message----- > > > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Clint > > > Taylor > > > Sent: Thursday, 24 October 2024 0.47 > > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx > > > Subject: [PATCH v2 07/12] drm/i915/cx0: Remove bus reset after every c10 > > > transaction > > > > > > C10 phy timeouts occur on xe3lpd if the c10 bus is reset every transaction. > > > Starting with xe3lpd this is bus reset not necessary > > > > > > > This C10/C20 bus reset was originally placed as a workaround to prevent bus timeouts. > > These timeouts were fixed elsewhere and therefore these are unnecessary lines. > > I'm a bit confused by the patch / explanation here. Before this patch > we did the reset on all platforms, unconditionally. The code change > below is removing the reset from the existing platforms (MTL/ARL and > Xe2) but keeping it only on the new Xe3 platforms. > > If the timeout mystery was solved and these resets are no longer needed, > shouldn't we be removing the line completely rather than making it > conditional to the new platforms? Or do we have now have new, > unexplained failures specifically on Xe3 that requires that we bring > back this hack at the same time we're removing it from the older > platforms? > I reversed the conditional when splitting the c10 patches. Will correct and send a new series. -Clint > > Matt > > > Reviewed-by: Mika Kahola <mika.kahola@xxxxxxxxx> > > > > > Signed-off-by: Clint Taylor <clinton.a.taylor@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > index c1357bdb8a3b..a8966a7a9927 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > @@ -224,7 +224,8 @@ static int __intel_cx0_read_once(struct intel_encoder > > > *encoder, > > > * down and let the message bus to end up > > > * in a known state > > > */ > > > - intel_cx0_bus_reset(encoder, lane); > > > + if ((DISPLAY_VER(i915) >= 30)) > > > + intel_cx0_bus_reset(encoder, lane); > > > > > > return REG_FIELD_GET(XELPDP_PORT_P2M_DATA_MASK, val); } @@ - > > > 313,7 +314,8 @@ static int __intel_cx0_write_once(struct intel_encoder > > > *encoder, > > > * down and let the message bus to end up > > > * in a known state > > > */ > > > - intel_cx0_bus_reset(encoder, lane); > > > + if ((DISPLAY_VER(i915) >= 30)) > > > + intel_cx0_bus_reset(encoder, lane); > > > > > > return 0; > > > } > > > -- > > > 2.25.1
