> -----Original Message----- > From: Li, Dennis <Dennis.Li@xxxxxxx> > Sent: Sunday, October 20, 2019 11:02 PM > To: Grodzovsky, Andrey <Andrey.Grodzovsky@xxxxxxx>; amd- > gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Grodzovsky, Andrey <Andrey.Grodzovsky@xxxxxxx>; Chen, Guchun > <Guchun.Chen@xxxxxxx>; Zhou1, Tao <Tao.Zhou1@xxxxxxx>; noreply- > confluence@xxxxxxx; Deucher, Alexander > <Alexander.Deucher@xxxxxxx>; Quan, Evan <Evan.Quan@xxxxxxx> > Subject: RE: [PATCH 2/4] drm/amd/powerplay: Add EEPROM I2C read/write > support to Arcturus. > > To make the function behavior more clear, It's better to change the function > declaration from > > + static void arcturus_fill_eeprom_i2c_req(SwI2cRequest_t *req, bool write, > + uint8_t address, uint32_t numbytes, > + uint8_t *data) > > to > > +static void arcturus_fill_eeprom_i2c_req(SwI2cRequest_t *req, bool > write_or_read, > + uint8_t address, uint32_t numbytes, > + uint8_t *data) I'm not sure I agree with that. If the variable is write_or_read then how does the driver code know how to set it? E.g., do you set it for write or for read or both? Alex > > Best Regards > Dennis Li > -----Original Message----- > From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of > Andrey Grodzovsky > Sent: Saturday, October 19, 2019 4:48 AM > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Grodzovsky, Andrey <Andrey.Grodzovsky@xxxxxxx>; Chen, Guchun > <Guchun.Chen@xxxxxxx>; Zhou1, Tao <Tao.Zhou1@xxxxxxx>; noreply- > confluence@xxxxxxx; Deucher, Alexander > <Alexander.Deucher@xxxxxxx>; Quan, Evan <Evan.Quan@xxxxxxx> > Subject: [PATCH 2/4] drm/amd/powerplay: Add EEPROM I2C read/write > support to Arcturus. > > The communication is done through SMU table and hence the code is in > powerplay. > > Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx> > --- > drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 229 > +++++++++++++++++++++++++++ > 1 file changed, 229 insertions(+) > > diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c > b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c > index 90d871a..53d08de5 100644 > --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c > +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c > @@ -36,6 +36,11 @@ > #include "smu_v11_0_pptable.h" > #include "arcturus_ppsmc.h" > #include "nbio/nbio_7_4_sh_mask.h" > +#include <linux/i2c.h> > +#include <linux/pci.h> > +#include "amdgpu_ras.h" > + > +#define to_amdgpu_device(x) (container_of(x, struct amdgpu_ras, > +eeprom_control.eeprom_accessor))->adev > > #define CTF_OFFSET_EDGE 5 > #define CTF_OFFSET_HOTSPOT 5 > @@ -171,6 +176,7 @@ static struct smu_11_0_cmn2aisc_mapping > arcturus_table_map[SMU_TABLE_COUNT] = { > TAB_MAP(SMU_METRICS), > TAB_MAP(DRIVER_SMU_CONFIG), > TAB_MAP(OVERDRIVE), > + TAB_MAP(I2C_COMMANDS), > }; > > static struct smu_11_0_cmn2aisc_mapping > arcturus_pwr_src_map[SMU_POWER_SOURCE_COUNT] = { @@ -293,6 > +299,9 @@ static int arcturus_tables_init(struct smu_context *smu, struct > smu_table *table > SMU_TABLE_INIT(tables, SMU_TABLE_SMU_METRICS, > sizeof(SmuMetrics_t), > PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM); > > + SMU_TABLE_INIT(tables, SMU_TABLE_I2C_COMMANDS, > sizeof(SwI2cRequest_t), > + PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM); > + > smu_table->metrics_table = kzalloc(sizeof(SmuMetrics_t), > GFP_KERNEL); > if (!smu_table->metrics_table) > return -ENOMEM; > @@ -1927,6 +1936,224 @@ static int arcturus_dpm_set_uvd_enable(struct > smu_context *smu, bool enable) > return ret; > } > > + > +static void arcturus_fill_eeprom_i2c_req(SwI2cRequest_t *req, bool write, > + uint8_t address, uint32_t numbytes, > + uint8_t *data) > +{ > + int i; > + > + BUG_ON(numbytes > MAX_SW_I2C_COMMANDS); > + > + req->I2CcontrollerPort = 0; > + req->I2CSpeed = 2; > + req->SlaveAddress = address; > + req->NumCmds = numbytes; > + > + for (i = 0; i < numbytes; i++) { > + SwI2cCmd_t *cmd = &req->SwI2cCmds[i]; > + > + /* First 2 bytes are always write for lower 2b EEPROM > address */ > + if (i < 2) > + cmd->Cmd = 1; > + else > + cmd->Cmd = write; > + > + > + /* Add RESTART for read after address filled */ > + cmd->CmdConfig |= (i == 2 && !write) ? > CMDCONFIG_RESTART_MASK : 0; > + > + /* Add STOP in the end */ > + cmd->CmdConfig |= (i == (numbytes - 1)) ? > CMDCONFIG_STOP_MASK : 0; > + > + /* Fill with data regardless if read or write to simplify code */ > + cmd->RegisterAddr = data[i]; > + } > +} > + > +static int arcturus_i2c_eeprom_read_data(struct i2c_adapter *control, > + uint8_t address, > + uint8_t *data, > + uint32_t numbytes) > +{ > + uint32_t i, ret = 0; > + SwI2cRequest_t req; > + struct amdgpu_device *adev = to_amdgpu_device(control); > + struct smu_table_context *smu_table = &adev->smu.smu_table; > + struct smu_table *table = &smu_table- > >tables[SMU_TABLE_I2C_COMMANDS]; > + > + memset(&req, 0, sizeof(req)); > + arcturus_fill_eeprom_i2c_req(&req, false, address, numbytes, data); > + > + mutex_lock(&adev->smu.mutex); > + /* Now read data starting with that address */ > + ret = smu_update_table(&adev->smu, > SMU_TABLE_I2C_COMMANDS, 0, &req, > + true); > + mutex_unlock(&adev->smu.mutex); > + > + if (!ret) { > + SwI2cRequest_t *res = (SwI2cRequest_t *)table->cpu_addr; > + > + /* Assume SMU fills res.SwI2cCmds[i].Data with read bytes > */ > + for (i = 0; i < numbytes; i++) > + data[i] = res->SwI2cCmds[i].Data; > + > + pr_debug("arcturus_i2c_eeprom_read_data, address = %x, > bytes = %d, data :", > + (uint16_t)address, numbytes); > + > + print_hex_dump(KERN_DEBUG, "data: ", > DUMP_PREFIX_NONE, > + 8, 1, data, numbytes, false); > + } else > + pr_err("arcturus_i2c_eeprom_read_data - error occurred > :%x", ret); > + > + return ret; > +} > + > +static int arcturus_i2c_eeprom_write_data(struct i2c_adapter *control, > + uint8_t address, > + uint8_t *data, > + uint32_t numbytes) > +{ > + uint32_t ret; > + SwI2cRequest_t req; > + struct amdgpu_device *adev = to_amdgpu_device(control); > + > + memset(&req, 0, sizeof(req)); > + arcturus_fill_eeprom_i2c_req(&req, true, address, numbytes, data); > + > + mutex_lock(&adev->smu.mutex); > + ret = smu_update_table(&adev->smu, > SMU_TABLE_I2C_COMMANDS, 0, &req, true); > + mutex_unlock(&adev->smu.mutex); > + > + if (!ret) { > + pr_debug("arcturus_i2c_write(), address = %x, bytes = %d , > data: ", > + (uint16_t)address, numbytes); > + > + print_hex_dump(KERN_DEBUG, "data: ", > DUMP_PREFIX_NONE, > + 8, 1, data, numbytes, false); > + /* > + * According to EEPROM spec there is a MAX of 10 ms > required for > + * EEPROM to flush internal RX buffer after STOP was issued > at the > + * end of write transaction. During this time the EEPROM will > not be > + * responsive to any more commands - so wait a bit more. > + */ > + msleep(10); > + > + } else > + pr_err("arcturus_i2c_write- error occurred :%x", ret); > + > + return ret; > +} > + > +static int arcturus_i2c_eeprom_i2c_xfer(struct i2c_adapter *i2c_adap, > + struct i2c_msg *msgs, int num) { > + uint32_t i, j, ret, data_size, data_chunk_size, next_eeprom_addr = > 0; > + uint8_t *data_ptr, data_chunk[MAX_SW_I2C_COMMANDS] = { 0 }; > + > + for (i = 0; i < num; i++) { > + /* > + * SMU interface allows at most MAX_SW_I2C_COMMANDS > bytes of data at > + * once and hence the data needs to be spliced into chunks > and sent each > + * chunk separately > + */ > + data_size = msgs[i].len - 2; > + data_chunk_size = MAX_SW_I2C_COMMANDS - 2; > + next_eeprom_addr = (msgs[i].buf[0] << 8 & 0xff00) | > (msgs[i].buf[1] & 0xff); > + data_ptr = msgs[i].buf + 2; > + > + for (j = 0; j < data_size / data_chunk_size; j++) { > + /* Insert the EEPROM dest addess, bits 0-15 */ > + data_chunk[0] = ((next_eeprom_addr >> 8) & 0xff); > + data_chunk[1] = (next_eeprom_addr & 0xff); > + > + if (msgs[i].flags & I2C_M_RD) { > + ret = > arcturus_i2c_eeprom_read_data(i2c_adap, > + > (uint8_t)msgs[i].addr, > + data_chunk, > MAX_SW_I2C_COMMANDS); > + > + memcpy(data_ptr, data_chunk + 2, > data_chunk_size); > + } else { > + > + memcpy(data_chunk + 2, data_ptr, > data_chunk_size); > + > + ret = > arcturus_i2c_eeprom_write_data(i2c_adap, > + > (uint8_t)msgs[i].addr, > + data_chunk, > MAX_SW_I2C_COMMANDS); > + } > + > + if (ret) { > + num = -EIO; > + goto fail; > + } > + > + next_eeprom_addr += data_chunk_size; > + data_ptr += data_chunk_size; > + } > + > + if (data_size % data_chunk_size) { > + data_chunk[0] = ((next_eeprom_addr >> 8) & 0xff); > + data_chunk[1] = (next_eeprom_addr & 0xff); > + > + if (msgs[i].flags & I2C_M_RD) { > + ret = > arcturus_i2c_eeprom_read_data(i2c_adap, > + > (uint8_t)msgs[i].addr, > + data_chunk, > (data_size % data_chunk_size) + 2); > + > + memcpy(data_ptr, data_chunk + 2, data_size > % data_chunk_size); > + } else { > + memcpy(data_chunk + 2, data_ptr, data_size > % data_chunk_size); > + > + ret = > arcturus_i2c_eeprom_write_data(i2c_adap, > + > (uint8_t)msgs[i].addr, > + data_chunk, > (data_size % data_chunk_size) + 2); > + } > + > + if (ret) { > + num = -EIO; > + goto fail; > + } > + } > + } > + > +fail: > + return num; > +} > + > +static u32 arcturus_i2c_eeprom_i2c_func(struct i2c_adapter *adap) { > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; } > + > + > +static const struct i2c_algorithm arcturus_i2c_eeprom_i2c_algo = { > + .master_xfer = arcturus_i2c_eeprom_i2c_xfer, > + .functionality = arcturus_i2c_eeprom_i2c_func, }; > + > +int arcturus_i2c_eeprom_control_init(struct i2c_adapter *control) { > + struct amdgpu_device *adev = to_amdgpu_device(control); > + int res; > + > + control->owner = THIS_MODULE; > + control->class = I2C_CLASS_SPD; > + control->dev.parent = &adev->pdev->dev; > + control->algo = &arcturus_i2c_eeprom_i2c_algo; > + snprintf(control->name, sizeof(control->name), "RAS EEPROM"); > + > + res = i2c_add_adapter(control); > + if (res) > + DRM_ERROR("Failed to register hw i2c, err: %d\n", res); > + > + return res; > +} > + > +void arcturus_i2c_eeprom_control_fini(struct i2c_adapter *control) { > + i2c_del_adapter(control); > +} > + > static const struct pptable_funcs arcturus_ppt_funcs = { > /* translate smu index into arcturus specific index */ > .get_smu_msg_index = arcturus_get_smu_msg_index, @@ -1966,6 > +2193,8 @@ static const struct pptable_funcs arcturus_ppt_funcs = { > .get_power_limit = arcturus_get_power_limit, > .is_dpm_running = arcturus_is_dpm_running, > .dpm_set_uvd_enable = arcturus_dpm_set_uvd_enable, > + .i2c_eeprom_init = arcturus_i2c_eeprom_control_init, > + .i2c_eeprom_fini = arcturus_i2c_eeprom_control_fini, > }; > > void arcturus_set_ppt_funcs(struct smu_context *smu) > -- > 2.7.4 > > _______________________________________________ > 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