> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Kahola, Mika > Sent: Thursday, November 2, 2023 4:54 PM > To: Sousa, Gustavo <gustavo.sousa@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Nikula, Jani <jani.nikula@xxxxxxxxx>; Syrjala, Ville <ville.syrjala@xxxxxxxxx> > Subject: Re: [PATCH] drm/i915/mtl: Clear possible sticky bits on PICA message bus > > > -----Original Message----- > > From: Sousa, Gustavo <gustavo.sousa@xxxxxxxxx> > > Sent: Thursday, November 2, 2023 4:23 PM > > To: Kahola, Mika <mika.kahola@xxxxxxxxx>; > > intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Kahola, Mika <mika.kahola@xxxxxxxxx>; Nikula, Jani > > <jani.nikula@xxxxxxxxx>; Syrjala, Ville <ville.syrjala@xxxxxxxxx> > > Subject: Re: [PATCH] drm/i915/mtl: Clear possible sticky bits on PICA > > message bus > > > > Quoting Mika Kahola (2023-11-01 07:31:01-03:00) > > >It is possible that sticky bits or error bits are left on message bus > > >status register. Reading and then writing the value back to > > >messagebus status register clears all possible sticky bits and errors. > > > > > >Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx> > > >--- > > > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > >b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > >index b2ad4c6172f6..f439f0c7b400 100644 > > >--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > >@@ -195,6 +195,13 @@ static int __intel_cx0_read_once(struct drm_i915_private *i915, enum port port, > > > return -ETIMEDOUT; > > > } > > > > > >+ /* > > >+ * write XELPDP_PORT_P2M_MSGBUS_STATUS register after read to clear > > >+ * any error sticky bits set from previous transactions > > >+ */ > > >+ val = intel_de_read(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane)); > > >+ intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, > > >+ lane), val); > > >+ > > > intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > > XELPDP_PORT_M2P_TRANSACTION_PENDING | > > > XELPDP_PORT_M2P_COMMAND_READ | @@ -262,6 > > >+269,13 @@ static int __intel_cx0_write_once(struct drm_i915_private > > >+*i915, enum port port, > > > return -ETIMEDOUT; > > > } > > > > > >+ /* > > >+ * write XELPDP_PORT_P2M_MSGBUS_STATUS register after read to clear > > >+ * any error sticky bits set from previous transactions > > >+ */ > > >+ val = intel_de_read(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane)); > > >+ intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, > > >+ lane), val); > > >+ > > > > Looking at the current state of the code, looks like to me that we already clear the bits from both the "success" and "failure" > paths. > > For the "success" > > paths, that is done by a direct call to > > intel_clear_response_ready_flag(); for the "failure" case, the call to > > intel_clear_response_ready_flag() is done as part of intel_cx0_bus_reset(). > > > > Thus, considering that we start using the msgbus from a clean state, > > maybe these extra steps are not necessary? Have you tried adding a > > call to > > intel_cx0_bus_reset() as part of intel_cx0_phy_transaction_begin()? > That I haven't try to reset bus at the stage. I can give it a go and check what happens. To me it seems, that we are sometimes > stuck at waiting the ack and eventually we time out and bail out. I have no clue why this happens from time to time. We already > reset the bus after every read/write operation. In addition, a small delay seem to help and clear the sporadic read/write failures > to the bus. However, this is more like a workaround and I'm not really happy about this sort of hack. > > I will give a go for your suggestion and come back once I have the results. I ran a test with intel_cx0_bus_reset() when placed in intel_cx0_phy_transaction_begin(). This didn't help either as with kms_flip IGT test I was able to trigger this read failure [drm] *ERROR* PHY G Read 0d80 failed after 3 retries. This was with the configuration where the test vehicle had 4K and eDP monitors connected. > > Thanks! > -Mika- > > > > > Also, I think it would be good if we understood better were those uncleared bits are coming from... > > > > -- > > Gustavo Sousa > > > > > intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > > XELPDP_PORT_M2P_TRANSACTION_PENDING | > > > (committed ? XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED : > > >-- > > >2.34.1 > > >
