[PATCH v2 07/12] drm/i915/cx0: Remove bus reset after every c10 transaction

[Matt Roper]

On Thu, Oct 24, 2024 at 06:08:46AM +0000, Kahola, Mika wrote:
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Clint
> > Taylor
> > Sent: Thursday, 24 October 2024 0.47
> > To: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> > 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 at intel.com>
> 
> > Signed-off-by: Clint Taylor <clinton.a.taylor at intel.com>
> > ---
> >  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