[PATCH 30/40] drm/amd/pm: Simplify managed I2C transfer functions
Alex Deucher
alexdeucher at gmail.com
Thu Jun 10 21:08:48 UTC 2021
On Tue, Jun 8, 2021 at 5:41 PM Luben Tuikov <luben.tuikov at amd.com> 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 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>
> ---
> .../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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list