On Wed, 01 Nov 2023, Mika Kahola <mika.kahola@xxxxxxxxx> wrote: > 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. Note that I don't know if this is the right thing to do, or the right place to do this, but I'll just comment on the *implementation*, i.e. please wait for proper review before addressing my comments. > > 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); I think it's slightly confusing to use val here, as it's then passed on to intel_cx0_wait_for_ack() and you're left wondering if that's required or just reuse of the val variable. This should do the same thing in one line, without reusing val: intel_de_rmw(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), 0, 0); > + > 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); > + > intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > XELPDP_PORT_M2P_TRANSACTION_PENDING | > (committed ? XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED : -- Jani Nikula, Intel
