> Subject: RE: [PATCH 3/3] drm/i915/cx0: Clear response ready & error bit > > Quoting Kandpal, Suraj (2025-12-31 01:59:29-03:00) > >> Subject: Re: [PATCH 3/3] drm/i915/cx0: Clear response ready & error > >> bit > >> > >> Quoting Suraj Kandpal (2025-12-30 05:31:42-03:00) > >> >Clear the response ready and error bit of > >> >PORT_P2M_MESSAGE_BUS_STATUS before writing the transaction > pending > >> >bit of PORT_M2P_MSGBUS_CTL as that is a hard requirement. If not > >> >done we find that the PHY hangs since it ends up in a weird state if left > idle for more than 1 hour. > >> > >> Since the series title refers to suspend/resume, is there an easy way > >> of reproducing this via some power state transition? > > > >Sadly no we did try to no avail went as low as DC9 too. Tried sleep > >then wake, Tried hibernating too. > > I see. I was wondering if DMC could be at play here. > > So the only known way of reproducing this is to leave it idle for more than 1 > hour? Do you know if that happens if we load the driver with > DC5/6 disabled (i.e. enable_dc=0)? > > > > >> > >> I'm wondering if we are looking at a driver issue here or if this is > >> really something else. I see that we usually call > >> intel_cx0_bus_reset() in error paths, which contains a call to > >> intel_clear_response_ready_flag(), but it could end up being not called if > the reset times out. > > > >Yes the reset times out and this is because PHY is in hanged state here > > > >> > >> Do we see error messages from the driver when the PHY hangs? > > > >Yes we do see a PHY hang when we are at the stage of swing programming > >setting And the error happens when we try to read the msg access bit > >itself which is because the PHY ends up in Weird state where it has a write > ack but no response ready bit getting set. > >But with my observation I can certainly say this is PHY issue. > >I also double confirmed this behavior with windows folks and their code > where they said they saw an issue in the same lines. > >Also the bspec basically adds this as a disclaimer that response ready > >and error bit should be cleared no matter what before writing The > transaction pending bit. > > Agreed on the need to clear the response ready bit before starting another > transaction, so the patch looks fine. I believe Jani's request to rename the > function wouldn't necessarily change the semantics of this patch, so > > Reviewed-by: Gustavo Sousa <[email protected]> > > in case the rename gets done as a follow-up. >
Thanks will separate out this patch from the series since the first patch seems to need a little more discussion Regards, Suraj Kandpal > > > >Regards, > >Suraj Kandpal > > > >> > >> -- > >> Gustavo Sousa > >> > >> > > >> >Bspec: 65101 > >> >Signed-off-by: Suraj Kandpal <[email protected]> > >> >--- > >> > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 4 ++++ > >> > 1 file changed, 4 insertions(+) > >> > > >> >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > >> >b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > >> >index 5edd293b533b..5ebc3404eee2 100644 > >> >--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > >> >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > >> >@@ -222,6 +222,8 @@ static int __intel_cx0_read_once(struct > >> >intel_encoder > >> *encoder, > >> > return -ETIMEDOUT; > >> > } > >> > > >> >+ intel_clear_response_ready_flag(encoder, lane); > >> >+ > >> > intel_de_write(display, XELPDP_PORT_M2P_MSGBUS_CTL(display, > >> > port, > >> lane), > >> > XELPDP_PORT_M2P_TRANSACTION_PENDING | > >> > XELPDP_PORT_M2P_COMMAND_READ | @@ -293,6 > >> >+295,8 @@ static int __intel_cx0_write_once(struct intel_encoder > *encoder, > >> > return -ETIMEDOUT; > >> > } > >> > > >> >+ intel_clear_response_ready_flag(encoder, lane); > >> >+ > >> > intel_de_write(display, XELPDP_PORT_M2P_MSGBUS_CTL(display, > >> > port, > >> lane), > >> > XELPDP_PORT_M2P_TRANSACTION_PENDING | > >> > (committed ? > >> XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED : > >> >-- > >> >2.34.1 > >> >
