[PATCH 18/40] drm/amdgpu: Fix Vega20 I2C to be agnostic (v2)

Alex Deucher alexdeucher at gmail.com
Thu Jun 10 20:43:34 UTC 2021


On Tue, Jun 8, 2021 at 5:40 PM Luben Tuikov <luben.tuikov at amd.com> wrote:
>
> Teach Vega20 I2C to be agnostic. Allow addressing
> different devices while the master holds the bus.
> Set STOP as per the controller's specification.
>
> v2: Qualify generating ReSTART before the 1st byte
>     of the message, when set by the caller, as
>     those functions are separated, as caught by
>     Andrey G.
>
> Cc: Jean Delvare <jdelvare at suse.de>
> Cc: Alexander Deucher <Alexander.Deucher at amd.com>
> Cc: Andrey Grodzovsky <Andrey.Grodzovsky at amd.com>
> Cc: Lijo Lazar <Lijo.Lazar at amd.com>
> Cc: Stanley Yang <Stanley.Yang at amd.com>
> Cc: Hawking Zhang <Hawking.Zhang at amd.com>
> Signed-off-by: Luben Tuikov <luben.tuikov at amd.com>

Acked-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c |   4 +-
>  drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 105 +++++++++++++--------
>  2 files changed, 69 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> index fe0e9b0c4d5a38..d02ea083a6c69b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> @@ -41,10 +41,10 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>                 },
>                 {
>                         .addr = slave_addr,
> -                       .flags = read ? I2C_M_RD: 0,
> +                       .flags = read ? I2C_M_RD : 0,
>                         .len = bytes,
>                         .buf = eeprom_buf,
> -               }
> +               },
>         };
>         int r;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> index 5a90d9351b22eb..b8d6d308fb06a0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> @@ -41,9 +41,7 @@
>  #define I2C_SW_TIMEOUT        8
>  #define I2C_ABORT             0x10
>
> -/* I2C transaction flags */
> -#define I2C_NO_STOP    1
> -#define I2C_RESTART    2
> +#define I2C_X_RESTART         BIT(31)
>
>  #define to_amdgpu_device(x) (container_of(x, struct amdgpu_device, pm.smu_i2c))
>
> @@ -205,9 +203,6 @@ static uint32_t smu_v11_0_i2c_poll_rx_status(struct i2c_adapter *control)
>         return ret;
>  }
>
> -
> -
> -
>  /**
>   * smu_v11_0_i2c_transmit - Send a block of data over the I2C bus to a slave device.
>   *
> @@ -252,21 +247,22 @@ static uint32_t smu_v11_0_i2c_transmit(struct i2c_adapter *control,
>                 reg = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_STATUS);
>                 if (REG_GET_FIELD(reg, CKSVII2C_IC_STATUS, TFNF)) {
>                         do {
> -                               reg = 0;
> -                               /*
> -                                * Prepare transaction, no need to set RESTART. I2C engine will send
> -                                * START as soon as it sees data in TXFIFO
> -                                */
> -                               if (bytes_sent == 0)
> -                                       reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, RESTART,
> -                                                           (i2c_flag & I2C_RESTART) ? 1 : 0);
>                                 reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, DAT, data[bytes_sent]);
>
> -                               /* determine if we need to send STOP bit or not */
> -                               if (numbytes == 1)
> -                                       /* Final transaction, so send stop unless I2C_NO_STOP */
> -                                       reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, STOP,
> -                                                           (i2c_flag & I2C_NO_STOP) ? 0 : 1);
> +                               /* Final message, final byte, must
> +                                * generate a STOP, to release the
> +                                * bus, i.e. don't hold SCL low.
> +                                */
> +                               if (numbytes == 1 && i2c_flag & I2C_M_STOP)
> +                                       reg = REG_SET_FIELD(reg,
> +                                                           CKSVII2C_IC_DATA_CMD,
> +                                                           STOP, 1);
> +
> +                               if (bytes_sent == 0 && i2c_flag & I2C_X_RESTART)
> +                                       reg = REG_SET_FIELD(reg,
> +                                                           CKSVII2C_IC_DATA_CMD,
> +                                                           RESTART, 1);
> +
>                                 /* Write */
>                                 reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, CMD, 0);
>                                 WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_DATA_CMD, reg);
> @@ -341,23 +337,21 @@ static uint32_t smu_v11_0_i2c_receive(struct i2c_adapter *control,
>
>                 smu_v11_0_i2c_clear_status(control);
>
> -
>                 /* Prepare transaction */
> -
> -               /* Each time we disable I2C, so this is not a restart */
> -               if (bytes_received == 0)
> -                       reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, RESTART,
> -                                           (i2c_flag & I2C_RESTART) ? 1 : 0);
> -
>                 reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, DAT, 0);
>                 /* Read */
>                 reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, CMD, 1);
>
> -               /* Transmitting last byte */
> -               if (numbytes == 1)
> -                       /* Final transaction, so send stop if requested */
> -                       reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, STOP,
> -                                           (i2c_flag & I2C_NO_STOP) ? 0 : 1);
> +               /* Final message, final byte, must generate a STOP
> +                * to release the bus, i.e. don't hold SCL low.
> +                */
> +               if (numbytes == 1 && i2c_flag & I2C_M_STOP)
> +                       reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD,
> +                                           STOP, 1);
> +
> +               if (bytes_received == 0 && i2c_flag & I2C_X_RESTART)
> +                       reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD,
> +                                           RESTART, 1);
>
>                 WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_DATA_CMD, reg);
>
> @@ -591,23 +585,59 @@ static const struct i2c_lock_operations smu_v11_0_i2c_i2c_lock_ops = {
>  };
>
>  static int smu_v11_0_i2c_xfer(struct i2c_adapter *i2c_adap,
> -                             struct i2c_msg *msgs, int num)
> +                             struct i2c_msg *msg, int num)
>  {
>         int i, ret;
> +       u16 addr, dir;
>
>         smu_v11_0_i2c_init(i2c_adap);
>
> +       /* From the client's point of view, this sequence of
> +        * messages-- the array i2c_msg *msg, is a single transaction
> +        * on the bus, starting with START and ending with STOP.
> +        *
> +        * The client is welcome to send any sequence of messages in
> +        * this array, as processing under this function here is
> +        * striving to be agnostic.
> +        *
> +        * Record the first address and direction we see. If either
> +        * changes for a subsequent message, generate ReSTART. The
> +        * DW_apb_i2c databook, v1.21a, specifies that ReSTART is
> +        * generated when the direction changes, with the default IP
> +        * block parameter settings, but it doesn't specify if ReSTART
> +        * is generated when the address changes (possibly...). We
> +        * don't rely on the default IP block parameter settings as
> +        * the block is shared and they may change.
> +        */
> +       if (num > 0) {
> +               addr = msg[0].addr;
> +               dir  = msg[0].flags & I2C_M_RD;
> +       }
> +
>         for (i = 0; i < num; i++) {
> -               uint32_t i2c_flag = ((msgs[i].flags & I2C_M_NOSTART) ? 0 : I2C_RESTART) ||
> -                                   (((msgs[i].flags & I2C_M_STOP) ? 0 : I2C_NO_STOP));
> +               u32 i2c_flag = 0;
>
> -               if (msgs[i].flags & I2C_M_RD)
> +               if (msg[i].addr != addr || (msg[i].flags ^ dir) & I2C_M_RD) {
> +                       addr = msg[i].addr;
> +                       dir  = msg[i].flags & I2C_M_RD;
> +                       i2c_flag |= I2C_X_RESTART;
> +               }
> +
> +               if (i == num - 1) {
> +                       /* Set the STOP bit on the last message, so
> +                        * that the IP block generates a STOP after
> +                        * the last byte of the message.
> +                        */
> +                       i2c_flag |= I2C_M_STOP;
> +               }
> +
> +               if (msg[i].flags & I2C_M_RD)
>                         ret = smu_v11_0_i2c_read_data(i2c_adap,
> -                                                     msgs + i,
> +                                                     msg + i,
>                                                       i2c_flag);
>                 else
>                         ret = smu_v11_0_i2c_write_data(i2c_adap,
> -                                                      msgs + i,
> +                                                      msg + i,
>                                                        i2c_flag);
>
>                 if (ret != I2C_OK) {
> @@ -625,7 +655,6 @@ static u32 smu_v11_0_i2c_func(struct i2c_adapter *adap)
>         return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
>  }
>
> -
>  static const struct i2c_algorithm smu_v11_0_i2c_algo = {
>         .master_xfer = smu_v11_0_i2c_xfer,
>         .functionality = smu_v11_0_i2c_func,
> --
> 2.32.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list