| Message ID | 20220929131747.2592092-2-mika.kahola@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/mtl: Add C10 phy support | expand |
On Thu, 29 Sep 2022, Mika Kahola <mika.kahola@intel.com> wrote: > From: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > > XELPDP has C10 and C20 phys from Synopsys to drive displays. Each phy > has a dedicated PIPE 5.2 Message bus for configuration. This message > bus is used to configure the phy internal registers. This looks like a silly intermediate step, adding a bunch of static functions with __maybe_unused, just to be modified again in the next patch. > > Bspec: 64599, 65100, 65101, 67610, 67636 > > Cc: Mika Kahola <mika.kahola@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > Signed-off-by: Mika Kahola <mika.kahola@intel.com> (v4) > --- > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 179 +++++++++++++++++++ > 1 file changed, 179 insertions(+) > create mode 100644 drivers/gpu/drm/i915/display/intel_cx0_phy.c > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > new file mode 100644 > index 000000000000..7930b0255cfa > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > @@ -0,0 +1,179 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2021 Intel Corporation > + */ > + > +#include "intel_de.h" > +#include "intel_uncore.h" Do you use anything from intel_uncore.h directly, or is it just intel_de.h? > + > +static void intel_cx0_bus_reset(struct drm_i915_private *i915, enum port port, int lane) > +{ > + enum phy phy = intel_port_to_phy(i915, port); > + > + /* Bring the phy to idle. */ > + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > + XELPDP_PORT_M2P_TRANSACTION_RESET); > + > + /* Wait for Idle Clear. */ > + if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > + XELPDP_PORT_M2P_TRANSACTION_RESET, > + XELPDP_MSGBUS_TIMEOUT_SLOW)) { > + drm_err_once(&i915->drm, "Failed to bring PHY %c to idle. \n", phy_name(phy)); > + return; > + } > + > + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), ~0); > + return; Unnecessary return statement. > +} > + > +__maybe_unused static u8 intel_cx0_read(struct drm_i915_private *i915, enum port port, > + int lane, u16 addr) > +{ > + enum phy phy = intel_port_to_phy(i915, port); > + u32 val = 0; > + int attempts = 0; > + > +retry: > + if (attempts == 3) { > + drm_err_once(&i915->drm, "PHY %c Read %04x failed after %d retries. Status: 0x%x\n", phy_name(phy), addr, attempts, val ?: 0); > + return 0; > + } The code looks like it would benefit from abstracting a non-retrying read function that returns errors, with this function doing the retry loop using a conventional for loop. There's four copy-pasted bits of error handling here that's just error prone. > + > + /* Wait for pending transactions.*/ > + if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > + XELPDP_PORT_M2P_TRANSACTION_PENDING, > + XELPDP_MSGBUS_TIMEOUT_SLOW)) { > + drm_dbg(&i915->drm, "PHY %c Timeout waiting for previous transaction to complete. Reset the bus and retry.\n", phy_name(phy)); drm_dbg_kms() throughout. > + attempts++; > + intel_cx0_bus_reset(i915, port, lane); > + goto retry; > + } > + > + /* Issue the read command. */ > + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > + XELPDP_PORT_M2P_TRANSACTION_PENDING | > + XELPDP_PORT_M2P_COMMAND_READ | > + XELPDP_PORT_M2P_ADDRESS(addr)); > + > + /* Wait for response ready. And read response.*/ > + if (__intel_wait_for_register(&i915->uncore, > + XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), > + XELPDP_PORT_P2M_RESPONSE_READY, > + XELPDP_PORT_P2M_RESPONSE_READY, > + XELPDP_MSGBUS_TIMEOUT_FAST_US, > + XELPDP_MSGBUS_TIMEOUT_SLOW, &val)) { > + drm_dbg(&i915->drm, "PHY %c Timeout waiting for Read response ACK. Status: 0x%x\n", phy_name(phy), val); > + attempts++; > + intel_cx0_bus_reset(i915, port, lane); > + goto retry; > + } > + > + /* Check for error. */ > + if (val & XELPDP_PORT_P2M_ERROR_SET) { > + drm_dbg(&i915->drm, "PHY %c Error occurred during read command. Status: 0x%x\n", phy_name(phy), val); > + attempts++; > + intel_cx0_bus_reset(i915, port, lane); > + goto retry; > + } > + > + /* Check for Read Ack. */ > + if (REG_FIELD_GET(XELPDP_PORT_P2M_COMMAND_TYPE_MASK, val) != > + XELPDP_PORT_P2M_COMMAND_READ_ACK) { > + drm_dbg(&i915->drm, "PHY %c Not a Read response. MSGBUS Status: 0x%x.\n", phy_name(phy), val); > + attempts++; > + intel_cx0_bus_reset(i915, port, lane); > + goto retry; > + } > + > + /* Clear Response Ready flag.*/ > + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), ~0); Blank line before return. > + return (u8)REG_FIELD_GET(XELPDP_PORT_P2M_DATA_MASK, val); Unnecessary cast. > +} > + > +static int intel_cx0_wait_cwrite_ack(struct drm_i915_private *i915, > + enum port port, int lane) > +{ > + enum phy phy = intel_port_to_phy(i915, port); > + u32 val; > + > + /* Check for write ack. */ > + if (__intel_wait_for_register(&i915->uncore, > + XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), > + XELPDP_PORT_P2M_RESPONSE_READY, > + XELPDP_PORT_P2M_RESPONSE_READY, > + XELPDP_MSGBUS_TIMEOUT_FAST_US, > + XELPDP_MSGBUS_TIMEOUT_SLOW, &val)) { > + drm_dbg(&i915->drm, "PHY %c Timeout waiting for Committed message ACK. Status: 0x%x\n", phy_name(phy), val); > + return -ETIMEDOUT; > + } > + > + if ((REG_FIELD_GET(XELPDP_PORT_P2M_COMMAND_TYPE_MASK, val) != > + XELPDP_PORT_P2M_COMMAND_WRITE_ACK) || val & XELPDP_PORT_P2M_ERROR_SET) { > + drm_dbg(&i915->drm, "PHY %c Unexpected ACK received. MSGBUS STATUS: 0x%x.\n", phy_name(phy), val); > + return -EINVAL; > + } This is also copy-paste duplicating the stuff in the previous function. So why isn't this function used there? > + > + return 0; > +} > + > +__maybe_unused static void intel_cx0_write(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 attempts = 0; > + > +retry: > + if (attempts == 3) { > + drm_err_once(&i915->drm, "PHY %c Write %04x failed after %d retries.\n", phy_name(phy), addr, attempts); > + return; > + } Same here with the retries as in the write. Have a lower level non-retrying write function, and handle the rewrites at a different abstraction level. > + > + /* Wait for pending transactions.*/ > + if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > + XELPDP_PORT_M2P_TRANSACTION_PENDING, > + XELPDP_MSGBUS_TIMEOUT_SLOW)) { > + drm_dbg(&i915->drm, "PHY %c Timeout waiting for previous transaction to complete. Reset the bus and retry.\n", phy_name(phy)); > + attempts++; > + intel_cx0_bus_reset(i915, port, lane); > + goto retry; > + } > + > + /* Issue the write command. */ > + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > + XELPDP_PORT_M2P_TRANSACTION_PENDING | > + (committed ? XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED : > + XELPDP_PORT_M2P_COMMAND_WRITE_UNCOMMITTED) | > + XELPDP_PORT_M2P_DATA(data) | > + XELPDP_PORT_M2P_ADDRESS(addr)); > + > + /* Check for error. */ > + if (committed) { > + if (intel_cx0_wait_cwrite_ack(i915, port, lane) < 0) { > + attempts++; > + intel_cx0_bus_reset(i915, port, lane); > + goto retry; > + } > + } else if ((intel_de_read(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(phy, lane)) & > + XELPDP_PORT_P2M_ERROR_SET)) { > + drm_dbg(&i915->drm, "PHY %c Error occurred during write command.\n", phy_name(phy)); > + attempts++; > + intel_cx0_bus_reset(i915, port, lane); > + goto retry; > + } > + > + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), ~0); > + > + return; Unnecessary return statement. > +} > + > +__maybe_unused static void intel_cx0_rmw(struct drm_i915_private *i915, enum port port, > + int lane, u16 addr, u8 clear, u8 set, bool committed) > +{ > + u8 old, val; > + > + old = intel_cx0_read(i915, port, lane, addr); > + val = (old & ~clear) | set; > + > + if (val != old) > + intel_cx0_write(i915, port, lane, addr, val, committed); > +}
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Friday, September 30, 2022 12:05 PM > To: Kahola, Mika <mika.kahola@intel.com>; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 1/5] drm/i915/mtl: Add Support for C10, C20 PHY > Message Bus > > On Thu, 29 Sep 2022, Mika Kahola <mika.kahola@intel.com> wrote: > > From: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > > > > XELPDP has C10 and C20 phys from Synopsys to drive displays. Each phy > > has a dedicated PIPE 5.2 Message bus for configuration. This message > > bus is used to configure the phy internal registers. > > This looks like a silly intermediate step, adding a bunch of static functions with > __maybe_unused, just to be modified again in the next patch. Yes, this was an intermediate step to get around gcc warn on unused functions. > > > > > Bspec: 64599, 65100, 65101, 67610, 67636 > > > > Cc: Mika Kahola <mika.kahola@intel.com> > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Uma Shankar <uma.shankar@intel.com> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> (v4) > > --- > > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 179 > > +++++++++++++++++++ > > 1 file changed, 179 insertions(+) > > create mode 100644 drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > new file mode 100644 > > index 000000000000..7930b0255cfa > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > @@ -0,0 +1,179 @@ > > +// SPDX-License-Identifier: MIT > > +/* > > + * Copyright © 2021 Intel Corporation */ > > + > > +#include "intel_de.h" > > +#include "intel_uncore.h" > > Do you use anything from intel_uncore.h directly, or is it just intel_de.h? I don't think this C10 patch series use intel_uncore.h directly. I have to double check that though. If not this intel_uncore.h is not needed. > > > + > > +static void intel_cx0_bus_reset(struct drm_i915_private *i915, enum > > +port port, int lane) { > > + enum phy phy = intel_port_to_phy(i915, port); > > + > > + /* Bring the phy to idle. */ > > + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > + XELPDP_PORT_M2P_TRANSACTION_RESET); > > + > > + /* Wait for Idle Clear. */ > > + if (intel_de_wait_for_clear(i915, > XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > + XELPDP_PORT_M2P_TRANSACTION_RESET, > > + XELPDP_MSGBUS_TIMEOUT_SLOW)) { > > + drm_err_once(&i915->drm, "Failed to bring PHY %c to idle. \n", > phy_name(phy)); > > + return; > > + } > > + > > + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), > ~0); > > + return; Yeah, true. > > Unnecessary return statement. > > > +} > > + > > +__maybe_unused static u8 intel_cx0_read(struct drm_i915_private *i915, > enum port port, > > + int lane, u16 addr) > > +{ > > + enum phy phy = intel_port_to_phy(i915, port); > > + u32 val = 0; > > + int attempts = 0; > > + > > +retry: > > + if (attempts == 3) { > > + drm_err_once(&i915->drm, "PHY %c Read %04x failed after %d > retries. Status: 0x%x\n", phy_name(phy), addr, attempts, val ?: 0); > > + return 0; > > + } > > The code looks like it would benefit from abstracting a non-retrying read > function that returns errors, with this function doing the retry loop using a > conventional for loop. Yes, I could do some tidying up here > > There's four copy-pasted bits of error handling here that's just error prone. > > > + > > + /* Wait for pending transactions.*/ > > + if (intel_de_wait_for_clear(i915, > XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > + > XELPDP_PORT_M2P_TRANSACTION_PENDING, > > + XELPDP_MSGBUS_TIMEOUT_SLOW)) { > > + drm_dbg(&i915->drm, "PHY %c Timeout waiting for previous > > +transaction to complete. Reset the bus and retry.\n", phy_name(phy)); > > drm_dbg_kms() throughout. > > > + attempts++; > > + intel_cx0_bus_reset(i915, port, lane); > > + goto retry; > > + } > > + > > + /* Issue the read command. */ > > + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > + XELPDP_PORT_M2P_TRANSACTION_PENDING | > > + XELPDP_PORT_M2P_COMMAND_READ | > > + XELPDP_PORT_M2P_ADDRESS(addr)); > > + > > + /* Wait for response ready. And read response.*/ > > + if (__intel_wait_for_register(&i915->uncore, > > + > XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), > > + XELPDP_PORT_P2M_RESPONSE_READY, > > + XELPDP_PORT_P2M_RESPONSE_READY, > > + XELPDP_MSGBUS_TIMEOUT_FAST_US, > > + XELPDP_MSGBUS_TIMEOUT_SLOW, &val)) { > > + drm_dbg(&i915->drm, "PHY %c Timeout waiting for Read > response ACK. Status: 0x%x\n", phy_name(phy), val); > > + attempts++; > > + intel_cx0_bus_reset(i915, port, lane); > > + goto retry; > > + } > > + > > + /* Check for error. */ > > + if (val & XELPDP_PORT_P2M_ERROR_SET) { > > + drm_dbg(&i915->drm, "PHY %c Error occurred during read > command. Status: 0x%x\n", phy_name(phy), val); > > + attempts++; > > + intel_cx0_bus_reset(i915, port, lane); > > + goto retry; > > + } > > + > > + /* Check for Read Ack. */ > > + if (REG_FIELD_GET(XELPDP_PORT_P2M_COMMAND_TYPE_MASK, val) > != > > + XELPDP_PORT_P2M_COMMAND_READ_ACK) { > > + drm_dbg(&i915->drm, "PHY %c Not a Read response. MSGBUS > Status: 0x%x.\n", phy_name(phy), val); > > + attempts++; > > + intel_cx0_bus_reset(i915, port, lane); > > + goto retry; > > + } > > + > > + /* Clear Response Ready flag.*/ > > + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), > ~0); > > Blank line before return. I will delete this line > > > + return (u8)REG_FIELD_GET(XELPDP_PORT_P2M_DATA_MASK, val); > > Unnecessary cast. Fixing it with next set of patches. > > > +} > > + > > +static int intel_cx0_wait_cwrite_ack(struct drm_i915_private *i915, > > + enum port port, int lane) > > +{ > > + enum phy phy = intel_port_to_phy(i915, port); > > + u32 val; > > + > > + /* Check for write ack. */ > > + if (__intel_wait_for_register(&i915->uncore, > > + > XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), > > + XELPDP_PORT_P2M_RESPONSE_READY, > > + XELPDP_PORT_P2M_RESPONSE_READY, > > + XELPDP_MSGBUS_TIMEOUT_FAST_US, > > + XELPDP_MSGBUS_TIMEOUT_SLOW, &val)) { > > + drm_dbg(&i915->drm, "PHY %c Timeout waiting for Committed > message ACK. Status: 0x%x\n", phy_name(phy), val); > > + return -ETIMEDOUT; > > + } > > + > > + if ((REG_FIELD_GET(XELPDP_PORT_P2M_COMMAND_TYPE_MASK, val) > != > > + XELPDP_PORT_P2M_COMMAND_WRITE_ACK) || val & > XELPDP_PORT_P2M_ERROR_SET) { > > + drm_dbg(&i915->drm, "PHY %c Unexpected ACK received. > MSGBUS STATUS: 0x%x.\n", phy_name(phy), val); > > + return -EINVAL; > > + } > > This is also copy-paste duplicating the stuff in the previous function. So why isn't > this function used there? This would benefit an own function. I will fix that in the next series of patches. > > > + > > + return 0; > > +} > > + > > +__maybe_unused static void intel_cx0_write(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 attempts = 0; > > + > > +retry: > > + if (attempts == 3) { > > + drm_err_once(&i915->drm, "PHY %c Write %04x failed after %d > retries.\n", phy_name(phy), addr, attempts); > > + return; > > + } > > Same here with the retries as in the write. Have a lower level non-retrying write > function, and handle the rewrites at a different abstraction level. I'll try to rephrase these. > > > + > > + /* Wait for pending transactions.*/ > > + if (intel_de_wait_for_clear(i915, > XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > + > XELPDP_PORT_M2P_TRANSACTION_PENDING, > > + XELPDP_MSGBUS_TIMEOUT_SLOW)) { > > + drm_dbg(&i915->drm, "PHY %c Timeout waiting for previous > transaction to complete. Reset the bus and retry.\n", phy_name(phy)); > > + attempts++; > > + intel_cx0_bus_reset(i915, port, lane); > > + goto retry; > > + } > > + > > + /* Issue the write command. */ > > + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), > > + XELPDP_PORT_M2P_TRANSACTION_PENDING | > > + (committed ? > XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED : > > + XELPDP_PORT_M2P_COMMAND_WRITE_UNCOMMITTED) > | > > + XELPDP_PORT_M2P_DATA(data) | > > + XELPDP_PORT_M2P_ADDRESS(addr)); > > + > > + /* Check for error. */ > > + if (committed) { > > + if (intel_cx0_wait_cwrite_ack(i915, port, lane) < 0) { > > + attempts++; > > + intel_cx0_bus_reset(i915, port, lane); > > + goto retry; > > + } > > + } else if ((intel_de_read(i915, > XELPDP_PORT_P2M_MSGBUS_STATUS(phy, lane)) & > > + XELPDP_PORT_P2M_ERROR_SET)) { > > + drm_dbg(&i915->drm, "PHY %c Error occurred during write > command.\n", phy_name(phy)); > > + attempts++; > > + intel_cx0_bus_reset(i915, port, lane); > > + goto retry; > > + } > > + > > + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), > ~0); > > + > > + return; > > Unnecessary return statement. Yes. Thanks for the comments and a review. I will try to address these finding with the next iteration of this patch series. -Mika- > > > +} > > + > > +__maybe_unused static void intel_cx0_rmw(struct drm_i915_private *i915, > enum port port, > > + int lane, u16 addr, u8 clear, u8 set, bool committed) { > > + u8 old, val; > > + > > + old = intel_cx0_read(i915, port, lane, addr); > > + val = (old & ~clear) | set; > > + > > + if (val != old) > > + intel_cx0_write(i915, port, lane, addr, val, committed); } > > -- > Jani Nikula, Intel Open Source Graphics Center
On Thu, Sep 29, 2022 at 04:17:43PM +0300, Mika Kahola wrote: >From: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > >XELPDP has C10 and C20 phys from Synopsys to drive displays. Each phy >has a dedicated PIPE 5.2 Message bus for configuration. This message >bus is used to configure the phy internal registers. > >Bspec: 64599, 65100, 65101, 67610, 67636 > >Cc: Mika Kahola <mika.kahola@intel.com> >Cc: Imre Deak <imre.deak@intel.com> >Cc: Uma Shankar <uma.shankar@intel.com> >Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> >Signed-off-by: Mika Kahola <mika.kahola@intel.com> (v4) >--- > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 179 +++++++++++++++++++ > 1 file changed, 179 insertions(+) > create mode 100644 drivers/gpu/drm/i915/display/intel_cx0_phy.c > >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >new file mode 100644 >index 000000000000..7930b0255cfa >--- /dev/null >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >@@ -0,0 +1,179 @@ >+// SPDX-License-Identifier: MIT >+/* >+ * Copyright © 2021 Intel Corporation >+ */ >+ >+#include "intel_de.h" >+#include "intel_uncore.h" >+ >+static void intel_cx0_bus_reset(struct drm_i915_private *i915, enum port port, int lane) >+{ >+ enum phy phy = intel_port_to_phy(i915, port); >+ >+ /* Bring the phy to idle. */ >+ intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), >+ XELPDP_PORT_M2P_TRANSACTION_RESET); >+ >+ /* Wait for Idle Clear. */ >+ if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), >+ XELPDP_PORT_M2P_TRANSACTION_RESET, >+ XELPDP_MSGBUS_TIMEOUT_SLOW)) { >+ drm_err_once(&i915->drm, "Failed to bring PHY %c to idle. \n", phy_name(phy)); >+ return; >+ } >+ >+ intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), ~0); >+ return; >+} >+ >+__maybe_unused static u8 intel_cx0_read(struct drm_i915_private *i915, enum port port, >+ int lane, u16 addr) >+{ >+ enum phy phy = intel_port_to_phy(i915, port); >+ u32 val = 0; >+ int attempts = 0; >+ >+retry: >+ if (attempts == 3) { >+ drm_err_once(&i915->drm, "PHY %c Read %04x failed after %d retries. Status: 0x%x\n", phy_name(phy), addr, attempts, val ?: 0); >+ return 0; >+ } >+ >+ /* Wait for pending transactions.*/ >+ if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), >+ XELPDP_PORT_M2P_TRANSACTION_PENDING, >+ XELPDP_MSGBUS_TIMEOUT_SLOW)) { >+ drm_dbg(&i915->drm, "PHY %c Timeout waiting for previous transaction to complete. Reset the bus and retry.\n", phy_name(phy)); >+ attempts++; >+ intel_cx0_bus_reset(i915, port, lane); >+ goto retry; >+ } >+ >+ /* Issue the read command. */ >+ intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), >+ XELPDP_PORT_M2P_TRANSACTION_PENDING | >+ XELPDP_PORT_M2P_COMMAND_READ | >+ XELPDP_PORT_M2P_ADDRESS(addr)); >+ >+ /* Wait for response ready. And read response.*/ >+ if (__intel_wait_for_register(&i915->uncore, >+ XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), >+ XELPDP_PORT_P2M_RESPONSE_READY, >+ XELPDP_PORT_P2M_RESPONSE_READY, >+ XELPDP_MSGBUS_TIMEOUT_FAST_US, >+ XELPDP_MSGBUS_TIMEOUT_SLOW, &val)) { >+ drm_dbg(&i915->drm, "PHY %c Timeout waiting for Read response ACK. Status: 0x%x\n", phy_name(phy), val); >+ attempts++; >+ intel_cx0_bus_reset(i915, port, lane); >+ goto retry; >+ } >+ >+ /* Check for error. */ >+ if (val & XELPDP_PORT_P2M_ERROR_SET) { >+ drm_dbg(&i915->drm, "PHY %c Error occurred during read command. Status: 0x%x\n", phy_name(phy), val); >+ attempts++; >+ intel_cx0_bus_reset(i915, port, lane); >+ goto retry; >+ } >+ >+ /* Check for Read Ack. */ >+ if (REG_FIELD_GET(XELPDP_PORT_P2M_COMMAND_TYPE_MASK, val) != >+ XELPDP_PORT_P2M_COMMAND_READ_ACK) { >+ drm_dbg(&i915->drm, "PHY %c Not a Read response. MSGBUS Status: 0x%x.\n", phy_name(phy), val); >+ attempts++; >+ intel_cx0_bus_reset(i915, port, lane); >+ goto retry; >+ } >+ >+ /* Clear Response Ready flag.*/ >+ intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), ~0); >+ return (u8)REG_FIELD_GET(XELPDP_PORT_P2M_DATA_MASK, val); >+} >+ >+static int intel_cx0_wait_cwrite_ack(struct drm_i915_private *i915, >+ enum port port, int lane) >+{ >+ enum phy phy = intel_port_to_phy(i915, port); >+ u32 val; >+ >+ /* Check for write ack. */ >+ if (__intel_wait_for_register(&i915->uncore, >+ XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), >+ XELPDP_PORT_P2M_RESPONSE_READY, >+ XELPDP_PORT_P2M_RESPONSE_READY, >+ XELPDP_MSGBUS_TIMEOUT_FAST_US, >+ XELPDP_MSGBUS_TIMEOUT_SLOW, &val)) { >+ drm_dbg(&i915->drm, "PHY %c Timeout waiting for Committed message ACK. Status: 0x%x\n", phy_name(phy), val); >+ return -ETIMEDOUT; >+ } >+ >+ if ((REG_FIELD_GET(XELPDP_PORT_P2M_COMMAND_TYPE_MASK, val) != >+ XELPDP_PORT_P2M_COMMAND_WRITE_ACK) || val & XELPDP_PORT_P2M_ERROR_SET) { >+ drm_dbg(&i915->drm, "PHY %c Unexpected ACK received. MSGBUS STATUS: 0x%x.\n", phy_name(phy), val); >+ return -EINVAL; >+ } >+ >+ return 0; >+} >+ >+__maybe_unused static void intel_cx0_write(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 attempts = 0; >+ >+retry: >+ if (attempts == 3) { >+ drm_err_once(&i915->drm, "PHY %c Write %04x failed after %d retries.\n", phy_name(phy), addr, attempts); >+ return; >+ } >+ >+ /* Wait for pending transactions.*/ >+ if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), >+ XELPDP_PORT_M2P_TRANSACTION_PENDING, >+ XELPDP_MSGBUS_TIMEOUT_SLOW)) { >+ drm_dbg(&i915->drm, "PHY %c Timeout waiting for previous transaction to complete. Reset the bus and retry.\n", phy_name(phy)); >+ attempts++; >+ intel_cx0_bus_reset(i915, port, lane); >+ goto retry; >+ } >+ >+ /* Issue the write command. */ >+ intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), >+ XELPDP_PORT_M2P_TRANSACTION_PENDING | >+ (committed ? XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED : >+ XELPDP_PORT_M2P_COMMAND_WRITE_UNCOMMITTED) | >+ XELPDP_PORT_M2P_DATA(data) | >+ XELPDP_PORT_M2P_ADDRESS(addr)); >+ >+ /* Check for error. */ >+ if (committed) { >+ if (intel_cx0_wait_cwrite_ack(i915, port, lane) < 0) { >+ attempts++; >+ intel_cx0_bus_reset(i915, port, lane); >+ goto retry; >+ } >+ } else if ((intel_de_read(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(phy, lane)) & wrong argument here to XELPDP_PORT_P2M_MSGBUS_STATUS(). It should be port, not phy. Lucas De Marchi
diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c new file mode 100644 index 000000000000..7930b0255cfa --- /dev/null +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2021 Intel Corporation + */ + +#include "intel_de.h" +#include "intel_uncore.h" + +static void intel_cx0_bus_reset(struct drm_i915_private *i915, enum port port, int lane) +{ + enum phy phy = intel_port_to_phy(i915, port); + + /* Bring the phy to idle. */ + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), + XELPDP_PORT_M2P_TRANSACTION_RESET); + + /* Wait for Idle Clear. */ + if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), + XELPDP_PORT_M2P_TRANSACTION_RESET, + XELPDP_MSGBUS_TIMEOUT_SLOW)) { + drm_err_once(&i915->drm, "Failed to bring PHY %c to idle. \n", phy_name(phy)); + return; + } + + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), ~0); + return; +} + +__maybe_unused static u8 intel_cx0_read(struct drm_i915_private *i915, enum port port, + int lane, u16 addr) +{ + enum phy phy = intel_port_to_phy(i915, port); + u32 val = 0; + int attempts = 0; + +retry: + if (attempts == 3) { + drm_err_once(&i915->drm, "PHY %c Read %04x failed after %d retries. Status: 0x%x\n", phy_name(phy), addr, attempts, val ?: 0); + return 0; + } + + /* Wait for pending transactions.*/ + if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), + XELPDP_PORT_M2P_TRANSACTION_PENDING, + XELPDP_MSGBUS_TIMEOUT_SLOW)) { + drm_dbg(&i915->drm, "PHY %c Timeout waiting for previous transaction to complete. Reset the bus and retry.\n", phy_name(phy)); + attempts++; + intel_cx0_bus_reset(i915, port, lane); + goto retry; + } + + /* Issue the read command. */ + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), + XELPDP_PORT_M2P_TRANSACTION_PENDING | + XELPDP_PORT_M2P_COMMAND_READ | + XELPDP_PORT_M2P_ADDRESS(addr)); + + /* Wait for response ready. And read response.*/ + if (__intel_wait_for_register(&i915->uncore, + XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), + XELPDP_PORT_P2M_RESPONSE_READY, + XELPDP_PORT_P2M_RESPONSE_READY, + XELPDP_MSGBUS_TIMEOUT_FAST_US, + XELPDP_MSGBUS_TIMEOUT_SLOW, &val)) { + drm_dbg(&i915->drm, "PHY %c Timeout waiting for Read response ACK. Status: 0x%x\n", phy_name(phy), val); + attempts++; + intel_cx0_bus_reset(i915, port, lane); + goto retry; + } + + /* Check for error. */ + if (val & XELPDP_PORT_P2M_ERROR_SET) { + drm_dbg(&i915->drm, "PHY %c Error occurred during read command. Status: 0x%x\n", phy_name(phy), val); + attempts++; + intel_cx0_bus_reset(i915, port, lane); + goto retry; + } + + /* Check for Read Ack. */ + if (REG_FIELD_GET(XELPDP_PORT_P2M_COMMAND_TYPE_MASK, val) != + XELPDP_PORT_P2M_COMMAND_READ_ACK) { + drm_dbg(&i915->drm, "PHY %c Not a Read response. MSGBUS Status: 0x%x.\n", phy_name(phy), val); + attempts++; + intel_cx0_bus_reset(i915, port, lane); + goto retry; + } + + /* Clear Response Ready flag.*/ + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), ~0); + return (u8)REG_FIELD_GET(XELPDP_PORT_P2M_DATA_MASK, val); +} + +static int intel_cx0_wait_cwrite_ack(struct drm_i915_private *i915, + enum port port, int lane) +{ + enum phy phy = intel_port_to_phy(i915, port); + u32 val; + + /* Check for write ack. */ + if (__intel_wait_for_register(&i915->uncore, + XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), + XELPDP_PORT_P2M_RESPONSE_READY, + XELPDP_PORT_P2M_RESPONSE_READY, + XELPDP_MSGBUS_TIMEOUT_FAST_US, + XELPDP_MSGBUS_TIMEOUT_SLOW, &val)) { + drm_dbg(&i915->drm, "PHY %c Timeout waiting for Committed message ACK. Status: 0x%x\n", phy_name(phy), val); + return -ETIMEDOUT; + } + + if ((REG_FIELD_GET(XELPDP_PORT_P2M_COMMAND_TYPE_MASK, val) != + XELPDP_PORT_P2M_COMMAND_WRITE_ACK) || val & XELPDP_PORT_P2M_ERROR_SET) { + drm_dbg(&i915->drm, "PHY %c Unexpected ACK received. MSGBUS STATUS: 0x%x.\n", phy_name(phy), val); + return -EINVAL; + } + + return 0; +} + +__maybe_unused static void intel_cx0_write(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 attempts = 0; + +retry: + if (attempts == 3) { + drm_err_once(&i915->drm, "PHY %c Write %04x failed after %d retries.\n", phy_name(phy), addr, attempts); + return; + } + + /* Wait for pending transactions.*/ + if (intel_de_wait_for_clear(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), + XELPDP_PORT_M2P_TRANSACTION_PENDING, + XELPDP_MSGBUS_TIMEOUT_SLOW)) { + drm_dbg(&i915->drm, "PHY %c Timeout waiting for previous transaction to complete. Reset the bus and retry.\n", phy_name(phy)); + attempts++; + intel_cx0_bus_reset(i915, port, lane); + goto retry; + } + + /* Issue the write command. */ + intel_de_write(i915, XELPDP_PORT_M2P_MSGBUS_CTL(port, lane), + XELPDP_PORT_M2P_TRANSACTION_PENDING | + (committed ? XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED : + XELPDP_PORT_M2P_COMMAND_WRITE_UNCOMMITTED) | + XELPDP_PORT_M2P_DATA(data) | + XELPDP_PORT_M2P_ADDRESS(addr)); + + /* Check for error. */ + if (committed) { + if (intel_cx0_wait_cwrite_ack(i915, port, lane) < 0) { + attempts++; + intel_cx0_bus_reset(i915, port, lane); + goto retry; + } + } else if ((intel_de_read(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(phy, lane)) & + XELPDP_PORT_P2M_ERROR_SET)) { + drm_dbg(&i915->drm, "PHY %c Error occurred during write command.\n", phy_name(phy)); + attempts++; + intel_cx0_bus_reset(i915, port, lane); + goto retry; + } + + intel_de_write(i915, XELPDP_PORT_P2M_MSGBUS_STATUS(port, lane), ~0); + + return; +} + +__maybe_unused static void intel_cx0_rmw(struct drm_i915_private *i915, enum port port, + int lane, u16 addr, u8 clear, u8 set, bool committed) +{ + u8 old, val; + + old = intel_cx0_read(i915, port, lane, addr); + val = (old & ~clear) | set; + + if (val != old) + intel_cx0_write(i915, port, lane, addr, val, committed); +}