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? 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 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
