| Message ID | 20230609122130.69794-1-mika.kahola@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/mtl: Cleanup usage of phy lane reset | expand |
Quoting Mika Kahola (2023-06-09 09:21:30-03:00) >From PICA message bus we wait for acknowledgment from >read/write commands. In case of an error, we reset the >bus for the next command. > >Current implementation ends up resetting message bus twice >in cases where error is not the timeout. Since, we only need >to reset message bus once, let's move reset to corresponding >timeout error and drop the excess reset function calls from >read/write functions. > >Signed-off-by: Mika Kahola <mika.kahola@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> >--- > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >index 0600fdcd06ef..f235df5646ed 100644 >--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >@@ -116,6 +116,7 @@ static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port, > XELPDP_MSGBUS_TIMEOUT_SLOW, val)) { > drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for message ACK. Status: 0x%x\n", > phy_name(phy), *val); >+ intel_cx0_bus_reset(i915, port, lane); > return -ETIMEDOUT; > } > >@@ -158,10 +159,8 @@ static int __intel_cx0_read_once(struct drm_i915_private *i915, enum port port, > XELPDP_PORT_M2P_ADDRESS(addr)); > > ack = intel_cx0_wait_for_ack(i915, port, XELPDP_PORT_P2M_COMMAND_READ_ACK, lane, &val); >- if (ack < 0) { >- intel_cx0_bus_reset(i915, port, lane); >+ if (ack < 0) > return ack; >- } > > intel_clear_response_ready_flag(i915, port, lane); > >@@ -202,6 +201,7 @@ static int __intel_cx0_write_once(struct drm_i915_private *i915, enum port port, > int lane, u16 addr, u8 data, bool committed) > { > enum phy phy = intel_port_to_phy(i915, port); >+ int ack; > u32 val; > > if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), >@@ -230,10 +230,9 @@ static int __intel_cx0_write_once(struct drm_i915_private *i915, enum port port, > } > > if (committed) { >- if (intel_cx0_wait_for_ack(i915, port, XELPDP_PORT_P2M_COMMAND_WRITE_ACK, lane, &val) < 0) { >- intel_cx0_bus_reset(i915, port, lane); >- return -EINVAL; >- } >+ ack = intel_cx0_wait_for_ack(i915, port, XELPDP_PORT_P2M_COMMAND_WRITE_ACK, lane, &val); >+ if (ack < 0) >+ return ack; > } else if ((intel_de_read(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane)) & > XELPDP_PORT_P2M_ERROR_SET)) { > drm_dbg_kms(&i915->drm, >-- >2.34.1 >
On Tue, 2023-06-13 at 14:25 -0300, Gustavo Sousa wrote: > Quoting Mika Kahola (2023-06-09 09:21:30-03:00) > > From PICA message bus we wait for acknowledgment from > > read/write commands. In case of an error, we reset the > > bus for the next command. > > > > Current implementation ends up resetting message bus twice > > in cases where error is not the timeout. Since, we only need > > to reset message bus once, let's move reset to corresponding > > timeout error and drop the excess reset function calls from > > read/write functions. > > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > > Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> This is now pushed. Thank you for the patch and the review. BR, Jouni Högander > > > --- > > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > index 0600fdcd06ef..f235df5646ed 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > @@ -116,6 +116,7 @@ static int intel_cx0_wait_for_ack(struct > > drm_i915_private *i915, enum port port, > > > > XELPDP_MSGBUS_TIMEOUT_SLOW, val)) { > > drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for > > message ACK. Status: 0x%x\n", > > phy_name(phy), *val); > > + intel_cx0_bus_reset(i915, port, lane); > > return -ETIMEDOUT; > > } > > > > @@ -158,10 +159,8 @@ static int __intel_cx0_read_once(struct > > drm_i915_private *i915, enum port port, > > XELPDP_PORT_M2P_ADDRESS(addr)); > > > > ack = intel_cx0_wait_for_ack(i915, port, > > XELPDP_PORT_P2M_COMMAND_READ_ACK, lane, &val); > > - if (ack < 0) { > > - intel_cx0_bus_reset(i915, port, lane); > > + if (ack < 0) > > return ack; > > - } > > > > intel_clear_response_ready_flag(i915, port, lane); > > > > @@ -202,6 +201,7 @@ static int __intel_cx0_write_once(struct > > drm_i915_private *i915, enum port port, > > int lane, u16 addr, u8 data, bool > > committed) > > { > > enum phy phy = intel_port_to_phy(i915, port); > > + int ack; > > u32 val; > > > > if (intel_de_wait_for_clear(i915, > > XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > @@ -230,10 +230,9 @@ static int __intel_cx0_write_once(struct > > drm_i915_private *i915, enum port port, > > } > > > > if (committed) { > > - if (intel_cx0_wait_for_ack(i915, port, > > XELPDP_PORT_P2M_COMMAND_WRITE_ACK, lane, &val) < 0) { > > - intel_cx0_bus_reset(i915, port, lane); > > - return -EINVAL; > > - } > > + ack = intel_cx0_wait_for_ack(i915, port, > > XELPDP_PORT_P2M_COMMAND_WRITE_ACK, lane, &val); > > + if (ack < 0) > > + return ack; > > } else if ((intel_de_read(i915, > > XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane)) & > > XELPDP_PORT_P2M_ERROR_SET)) { > > drm_dbg_kms(&i915->drm, > > -- > > 2.34.1 > >
diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c index 0600fdcd06ef..f235df5646ed 100644 --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c @@ -116,6 +116,7 @@ static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port, XELPDP_MSGBUS_TIMEOUT_SLOW, val)) { drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for message ACK. Status: 0x%x\n", phy_name(phy), *val); + intel_cx0_bus_reset(i915, port, lane); return -ETIMEDOUT; } @@ -158,10 +159,8 @@ static int __intel_cx0_read_once(struct drm_i915_private *i915, enum port port, XELPDP_PORT_M2P_ADDRESS(addr)); ack = intel_cx0_wait_for_ack(i915, port, XELPDP_PORT_P2M_COMMAND_READ_ACK, lane, &val); - if (ack < 0) { - intel_cx0_bus_reset(i915, port, lane); + if (ack < 0) return ack; - } intel_clear_response_ready_flag(i915, port, lane); @@ -202,6 +201,7 @@ static int __intel_cx0_write_once(struct drm_i915_private *i915, enum port port, int lane, u16 addr, u8 data, bool committed) { enum phy phy = intel_port_to_phy(i915, port); + int ack; u32 val; if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), @@ -230,10 +230,9 @@ static int __intel_cx0_write_once(struct drm_i915_private *i915, enum port port, } if (committed) { - if (intel_cx0_wait_for_ack(i915, port, XELPDP_PORT_P2M_COMMAND_WRITE_ACK, lane, &val) < 0) { - intel_cx0_bus_reset(i915, port, lane); - return -EINVAL; - } + ack = intel_cx0_wait_for_ack(i915, port, XELPDP_PORT_P2M_COMMAND_WRITE_ACK, lane, &val); + if (ack < 0) + return ack; } else if ((intel_de_read(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane)) & XELPDP_PORT_P2M_ERROR_SET)) { drm_dbg_kms(&i915->drm,
From PICA message bus we wait for acknowledgment from read/write commands. In case of an error, we reset the bus for the next command. Current implementation ends up resetting message bus twice in cases where error is not the timeout. Since, we only need to reset message bus once, let's move reset to corresponding timeout error and drop the excess reset function calls from read/write functions. Signed-off-by: Mika Kahola <mika.kahola@intel.com> --- drivers/gpu/drm/i915/display/intel_cx0_phy.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)