Re: [PATCH 30/40] drm/amd/pm: Simplify managed I2C transfer functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 8, 2021 at 5:41 PM Luben Tuikov <luben.tuikov@xxxxxxx> wrote:
>
> Now that we have an I2C quirk table for
> SMU-managed I2C controllers, the I2C core does the
> checks for us, so we don't need to do them, and so
> simplify the managed I2C transfer functions.
>
> Also, for Arcturus and Navi10, fix setting the
> command type from "cmd->CmdConfig" to "cmd->Cmd".
> The latter is what appears to be taking in
> the enumeration I2C_CMD_... as an integer,
> not a bit-flag.
>
> For Sienna, the "Cmd" field seems to have been
> eliminated, and command type and flags all live in
> the "CmdConfig" field--this is left untouched.
>
> Fix: Detect and add changing of direction
> bit-flag, as this is necessary for the SMU to
> detect the direction change in the 1-d array of
> data it gets.
>
> Cc: Jean Delvare <jdelvare@xxxxxxx>
> Cc: Alexander Deucher <Alexander.Deucher@xxxxxxx>
> Cc: Andrey Grodzovsky <Andrey.Grodzovsky@xxxxxxx>
> Cc: Lijo Lazar <Lijo.Lazar@xxxxxxx>
> Cc: Stanley Yang <Stanley.Yang@xxxxxxx>
> Cc: Hawking Zhang <Hawking.Zhang@xxxxxxx>
> Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx>

Acked-by: Alex Deucher <alexander.deucher@xxxxxxx>

> ---
>  .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 78 ++++++++-----------
>  .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 78 ++++++++-----------
>  .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 76 ++++++++----------
>  3 files changed, 95 insertions(+), 137 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> index de8d7513042966..0db79a5236e1f1 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> @@ -1907,31 +1907,14 @@ static int arcturus_dpm_set_vcn_enable(struct smu_context *smu, bool enable)
>  }
>
>  static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap,
> -                            struct i2c_msg *msgs, int num)
> +                            struct i2c_msg *msg, int num_msgs)
>  {
>         struct amdgpu_device *adev = to_amdgpu_device(i2c_adap);
>         struct smu_table_context *smu_table = &adev->smu.smu_table;
>         struct smu_table *table = &smu_table->driver_table;
>         SwI2cRequest_t *req, *res = (SwI2cRequest_t *)table->cpu_addr;
> -       short available_bytes = MAX_SW_I2C_COMMANDS;
> -       int i, j, r, c, num_done = 0;
> -       u8 slave;
> -
> -       /* only support a single slave addr per transaction */
> -       slave = msgs[0].addr;
> -       for (i = 0; i < num; i++) {
> -               if (slave != msgs[i].addr)
> -                       return -EINVAL;
> -
> -               available_bytes -= msgs[i].len;
> -               if (available_bytes >= 0) {
> -                       num_done++;
> -               } else {
> -                       /* This message and all the follwing won't be processed */
> -                       available_bytes += msgs[i].len;
> -                       break;
> -               }
> -       }
> +       int i, j, r, c;
> +       u16 dir;
>
>         req = kzalloc(sizeof(*req), GFP_KERNEL);
>         if (!req)
> @@ -1939,33 +1922,38 @@ static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap,
>
>         req->I2CcontrollerPort = 1;
>         req->I2CSpeed = I2C_SPEED_FAST_400K;
> -       req->SlaveAddress = slave << 1; /* 8 bit addresses */
> -       req->NumCmds = MAX_SW_I2C_COMMANDS - available_bytes;;
> -
> -       c = 0;
> -       for (i = 0; i < num_done; i++) {
> -               struct i2c_msg *msg = &msgs[i];
> +       req->SlaveAddress = msg[0].addr << 1; /* wants an 8-bit address */
> +       dir = msg[0].flags & I2C_M_RD;
>
> -               for (j = 0; j < msg->len; j++) {
> -                       SwI2cCmd_t *cmd = &req->SwI2cCmds[c++];
> +       for (c = i = 0; i < num_msgs; i++) {
> +               for (j = 0; j < msg[i].len; j++, c++) {
> +                       SwI2cCmd_t *cmd = &req->SwI2cCmds[c];
>
>                         if (!(msg[i].flags & I2C_M_RD)) {
>                                 /* write */
> -                               cmd->CmdConfig |= I2C_CMD_WRITE;
> -                               cmd->RegisterAddr = msg->buf[j];
> +                               cmd->Cmd = I2C_CMD_WRITE;
> +                               cmd->RegisterAddr = msg[i].buf[j];
> +                       }
> +
> +                       if ((dir ^ msg[i].flags) & I2C_M_RD) {
> +                               /* The direction changes.
> +                                */
> +                               dir = msg[i].flags & I2C_M_RD;
> +                               cmd->CmdConfig |= CMDCONFIG_RESTART_MASK;
>                         }
>
> +                       req->NumCmds++;
> +
>                         /*
>                          * Insert STOP if we are at the last byte of either last
>                          * message for the transaction or the client explicitly
>                          * requires a STOP at this particular message.
>                          */
> -                       if ((j == msg->len -1 ) &&
> -                           ((i == num_done - 1) || (msg[i].flags & I2C_M_STOP)))
> +                       if ((j == msg[i].len - 1) &&
> +                           ((i == num_msgs - 1) || (msg[i].flags & I2C_M_STOP))) {
> +                               cmd->CmdConfig &= ~CMDCONFIG_RESTART_MASK;
>                                 cmd->CmdConfig |= CMDCONFIG_STOP_MASK;
> -
> -                       if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART))
> -                               cmd->CmdConfig |= CMDCONFIG_RESTART_BIT;
> +                       }
>                 }
>         }
>         mutex_lock(&adev->smu.mutex);
> @@ -1974,22 +1962,20 @@ static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap,
>         if (r)
>                 goto fail;
>
> -       c = 0;
> -       for (i = 0; i < num_done; i++) {
> -               struct i2c_msg *msg = &msgs[i];
> -
> -               for (j = 0; j < msg->len; j++) {
> -                       SwI2cCmd_t *cmd = &res->SwI2cCmds[c++];
> +       for (c = i = 0; i < num_msgs; i++) {
> +               if (!(msg[i].flags & I2C_M_RD)) {
> +                       c += msg[i].len;
> +                       continue;
> +               }
> +               for (j = 0; j < msg[i].len; j++, c++) {
> +                       SwI2cCmd_t *cmd = &res->SwI2cCmds[c];
>
> -                       if (msg[i].flags & I2C_M_RD)
> -                               msg->buf[j] = cmd->Data;
> +                       msg[i].buf[j] = cmd->Data;
>                 }
>         }
> -       r = num_done;
> -
> +       r = num_msgs;
>  fail:
>         kfree(req);
> -
>         return r;
>  }
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 1b8cd3746d0ebc..2acf54967c6ab1 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -2702,31 +2702,14 @@ static ssize_t navi10_get_legacy_gpu_metrics(struct smu_context *smu,
>  }
>
>  static int navi10_i2c_xfer(struct i2c_adapter *i2c_adap,
> -                          struct i2c_msg *msgs, int num)
> +                          struct i2c_msg *msg, int num_msgs)
>  {
>         struct amdgpu_device *adev = to_amdgpu_device(i2c_adap);
>         struct smu_table_context *smu_table = &adev->smu.smu_table;
>         struct smu_table *table = &smu_table->driver_table;
>         SwI2cRequest_t *req, *res = (SwI2cRequest_t *)table->cpu_addr;
> -       short available_bytes = MAX_SW_I2C_COMMANDS;
> -       int i, j, r, c, num_done = 0;
> -       u8 slave;
> -
> -       /* only support a single slave addr per transaction */
> -       slave = msgs[0].addr;
> -       for (i = 0; i < num; i++) {
> -               if (slave != msgs[i].addr)
> -                       return -EINVAL;
> -
> -               available_bytes -= msgs[i].len;
> -               if (available_bytes >= 0) {
> -                       num_done++;
> -               } else {
> -                       /* This message and all the follwing won't be processed */
> -                       available_bytes += msgs[i].len;
> -                       break;
> -               }
> -       }
> +       int i, j, r, c;
> +       u16 dir;
>
>         req = kzalloc(sizeof(*req), GFP_KERNEL);
>         if (!req)
> @@ -2734,33 +2717,38 @@ static int navi10_i2c_xfer(struct i2c_adapter *i2c_adap,
>
>         req->I2CcontrollerPort = 1;
>         req->I2CSpeed = I2C_SPEED_FAST_400K;
> -       req->SlaveAddress = slave << 1; /* 8 bit addresses */
> -       req->NumCmds = MAX_SW_I2C_COMMANDS - available_bytes;;
> +       req->SlaveAddress = msg[0].addr << 1; /* wants an 8-bit address */
> +       dir = msg[0].flags & I2C_M_RD;
>
> -       c = 0;
> -       for (i = 0; i < num_done; i++) {
> -               struct i2c_msg *msg = &msgs[i];
> -
> -               for (j = 0; j < msg->len; j++) {
> -                       SwI2cCmd_t *cmd = &req->SwI2cCmds[c++];
> +       for (c = i = 0; i < num_msgs; i++) {
> +               for (j = 0; j < msg[i].len; j++, c++) {
> +                       SwI2cCmd_t *cmd = &req->SwI2cCmds[c];
>
>                         if (!(msg[i].flags & I2C_M_RD)) {
>                                 /* write */
> -                               cmd->CmdConfig |= I2C_CMD_WRITE;
> -                               cmd->RegisterAddr = msg->buf[j];
> +                               cmd->Cmd = I2C_CMD_WRITE;
> +                               cmd->RegisterAddr = msg[i].buf[j];
> +                       }
> +
> +                       if ((dir ^ msg[i].flags) & I2C_M_RD) {
> +                               /* The direction changes.
> +                                */
> +                               dir = msg[i].flags & I2C_M_RD;
> +                               cmd->CmdConfig |= CMDCONFIG_RESTART_MASK;
>                         }
>
> +                       req->NumCmds++;
> +
>                         /*
>                          * Insert STOP if we are at the last byte of either last
>                          * message for the transaction or the client explicitly
>                          * requires a STOP at this particular message.
>                          */
> -                       if ((j == msg->len -1 ) &&
> -                           ((i == num_done - 1) || (msg[i].flags & I2C_M_STOP)))
> +                       if ((j == msg[i].len - 1) &&
> +                           ((i == num_msgs - 1) || (msg[i].flags & I2C_M_STOP))) {
> +                               cmd->CmdConfig &= ~CMDCONFIG_RESTART_MASK;
>                                 cmd->CmdConfig |= CMDCONFIG_STOP_MASK;
> -
> -                       if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART))
> -                               cmd->CmdConfig |= CMDCONFIG_RESTART_BIT;
> +                       }
>                 }
>         }
>         mutex_lock(&adev->smu.mutex);
> @@ -2769,22 +2757,20 @@ static int navi10_i2c_xfer(struct i2c_adapter *i2c_adap,
>         if (r)
>                 goto fail;
>
> -       c = 0;
> -       for (i = 0; i < num_done; i++) {
> -               struct i2c_msg *msg = &msgs[i];
> -
> -               for (j = 0; j < msg->len; j++) {
> -                       SwI2cCmd_t *cmd = &res->SwI2cCmds[c++];
> +       for (c = i = 0; i < num_msgs; i++) {
> +               if (!(msg[i].flags & I2C_M_RD)) {
> +                       c += msg[i].len;
> +                       continue;
> +               }
> +               for (j = 0; j < msg[i].len; j++, c++) {
> +                       SwI2cCmd_t *cmd = &res->SwI2cCmds[c];
>
> -                       if (msg[i].flags & I2C_M_RD)
> -                               msg->buf[j] = cmd->Data;
> +                       msg[i].buf[j] = cmd->Data;
>                 }
>         }
> -       r = num_done;
> -
> +       r = num_msgs;
>  fail:
>         kfree(req);
> -
>         return r;
>  }
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> index b38127f8009d3d..44ca3b3f83f4d9 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> @@ -3390,31 +3390,14 @@ static void sienna_cichlid_dump_pptable(struct smu_context *smu)
>  }
>
>  static int sienna_cichlid_i2c_xfer(struct i2c_adapter *i2c_adap,
> -                                  struct i2c_msg *msgs, int num)
> +                                  struct i2c_msg *msg, int num_msgs)
>  {
>         struct amdgpu_device *adev = to_amdgpu_device(i2c_adap);
>         struct smu_table_context *smu_table = &adev->smu.smu_table;
>         struct smu_table *table = &smu_table->driver_table;
>         SwI2cRequest_t *req, *res = (SwI2cRequest_t *)table->cpu_addr;
> -       short available_bytes = MAX_SW_I2C_COMMANDS;
> -       int i, j, r, c, num_done = 0;
> -       u8 slave;
> -
> -       /* only support a single slave addr per transaction */
> -       slave = msgs[0].addr;
> -       for (i = 0; i < num; i++) {
> -               if (slave != msgs[i].addr)
> -                       return -EINVAL;
> -
> -               available_bytes -= msgs[i].len;
> -               if (available_bytes >= 0) {
> -                       num_done++;
> -               } else {
> -                       /* This message and all the follwing won't be processed */
> -                       available_bytes += msgs[i].len;
> -                       break;
> -               }
> -       }
> +       int i, j, r, c;
> +       u16 dir;
>
>         req = kzalloc(sizeof(*req), GFP_KERNEL);
>         if (!req)
> @@ -3422,33 +3405,38 @@ static int sienna_cichlid_i2c_xfer(struct i2c_adapter *i2c_adap,
>
>         req->I2CcontrollerPort = 1;
>         req->I2CSpeed = I2C_SPEED_FAST_400K;
> -       req->SlaveAddress = slave << 1; /* 8 bit addresses */
> -       req->NumCmds = MAX_SW_I2C_COMMANDS - available_bytes;;
> +       req->SlaveAddress = msg[0].addr << 1; /* wants an 8-bit address */
> +       dir = msg[0].flags & I2C_M_RD;
>
> -       c = 0;
> -       for (i = 0; i < num_done; i++) {
> -               struct i2c_msg *msg = &msgs[i];
> -
> -               for (j = 0; j < msg->len; j++) {
> -                       SwI2cCmd_t *cmd = &req->SwI2cCmds[c++];
> +       for (c = i = 0; i < num_msgs; i++) {
> +               for (j = 0; j < msg[i].len; j++, c++) {
> +                       SwI2cCmd_t *cmd = &req->SwI2cCmds[c];
>
>                         if (!(msg[i].flags & I2C_M_RD)) {
>                                 /* write */
>                                 cmd->CmdConfig |= CMDCONFIG_READWRITE_MASK;
> -                               cmd->ReadWriteData = msg->buf[j];
> +                               cmd->ReadWriteData = msg[i].buf[j];
> +                       }
> +
> +                       if ((dir ^ msg[i].flags) & I2C_M_RD) {
> +                               /* The direction changes.
> +                                */
> +                               dir = msg[i].flags & I2C_M_RD;
> +                               cmd->CmdConfig |= CMDCONFIG_RESTART_MASK;
>                         }
>
> +                       req->NumCmds++;
> +
>                         /*
>                          * Insert STOP if we are at the last byte of either last
>                          * message for the transaction or the client explicitly
>                          * requires a STOP at this particular message.
>                          */
> -                       if ((j == msg->len -1 ) &&
> -                           ((i == num_done - 1) || (msg[i].flags & I2C_M_STOP)))
> +                       if ((j == msg[i].len - 1) &&
> +                           ((i == num_msgs - 1) || (msg[i].flags & I2C_M_STOP))) {
> +                               cmd->CmdConfig &= ~CMDCONFIG_RESTART_MASK;
>                                 cmd->CmdConfig |= CMDCONFIG_STOP_MASK;
> -
> -                       if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART))
> -                               cmd->CmdConfig |= CMDCONFIG_RESTART_BIT;
> +                       }
>                 }
>         }
>         mutex_lock(&adev->smu.mutex);
> @@ -3457,22 +3445,20 @@ static int sienna_cichlid_i2c_xfer(struct i2c_adapter *i2c_adap,
>         if (r)
>                 goto fail;
>
> -       c = 0;
> -       for (i = 0; i < num_done; i++) {
> -               struct i2c_msg *msg = &msgs[i];
> -
> -               for (j = 0; j < msg->len; j++) {
> -                       SwI2cCmd_t *cmd = &res->SwI2cCmds[c++];
> +       for (c = i = 0; i < num_msgs; i++) {
> +               if (!(msg[i].flags & I2C_M_RD)) {
> +                       c += msg[i].len;
> +                       continue;
> +               }
> +               for (j = 0; j < msg[i].len; j++, c++) {
> +                       SwI2cCmd_t *cmd = &res->SwI2cCmds[c];
>
> -                       if (msg[i].flags & I2C_M_RD)
> -                               msg->buf[j] = cmd->ReadWriteData;
> +                       msg[i].buf[j] = cmd->ReadWriteData;
>                 }
>         }
> -       r = num_done;
> -
> +       r = num_msgs;
>  fail:
>         kfree(req);
> -
>         return r;
>  }
>
> --
> 2.32.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux