On Fri, Mar 13, 2020 at 10:57 AM Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx> wrote: > > Puts the i2c adapter in common place for sharing by RAS > and upcoming nvmlDeviceGetSerial from FRU EEPROM feature. > > Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 35 +++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 48 +++++--------------------- > drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h | 2 -- > drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 14 ++++---- > drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 2 +- > 6 files changed, 55 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index c00831f..f807237 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -979,6 +979,9 @@ struct amdgpu_device { > > bool pm_sysfs_en; > bool ucode_sysfs_en; > + > + /* Used for I2C access to various EEPROMs on relevant ASICs */ > + struct i2c_adapter eeprom_accessor; Maybe call this smu_i2c? Since it's not really eeprom only. Also maybe put it in struct amdgpu_pm? > }; > > static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index a35c899..80fcc16 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -68,6 +68,8 @@ > #include <linux/suspend.h> > #include <drm/task_barrier.h> > > +#include "smu_v11_0_i2c.h" > + > MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin"); > MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin"); > MODULE_FIRMWARE("amdgpu/raven_gpu_info.bin"); > @@ -1848,6 +1850,33 @@ static int amdgpu_device_fw_loading(struct amdgpu_device *adev) > return r; > } > > +static int amdgpu_eeprom_init(struct amdgpu_device *adev) > +{ > + switch (adev->asic_type) { > + case CHIP_VEGA20: > + return smu_v11_0_i2c_eeprom_control_init(&adev->eeprom_accessor); > + case CHIP_ARCTURUS: > + return smu_i2c_eeprom_init(&adev->smu, &adev->eeprom_accessor); > + default: > + return 0; > + } > +} > + > +void amdgpu_eeprom_fini(struct amdgpu_device *adev) > +{ > + > + switch (adev->asic_type) { > + case CHIP_VEGA20: > + smu_v11_0_i2c_eeprom_control_fini(&adev->eeprom_accessor); > + return; > + case CHIP_ARCTURUS: > + smu_i2c_eeprom_fini(&adev->smu, &adev->eeprom_accessor); > + return; > + default: > + return; > + } > +} > + Please put these in a separate amdgpu_eeprom.c/h files or call their contents directly from their respective powerplay SMU files for each asic since they are tied to SMU init anyway. > /** > * amdgpu_device_ip_init - run init for hardware IPs > * > @@ -1936,6 +1965,10 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) > if (r) > goto init_failed; > > + r = amdgpu_eeprom_init(adev); > + if (r) > + goto init_failed; > + > /* > * retired pages will be loaded from eeprom and reserved here, > * it should be called after amdgpu_device_ip_hw_init_phase2 since > @@ -2196,6 +2229,8 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev) > > amdgpu_ras_pre_fini(adev); > > + amdgpu_eeprom_fini(adev); > + > if (adev->gmc.xgmi.num_physical_nodes > 1) > amdgpu_xgmi_remove_device(adev); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c > index ed15b1f..14f6571 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c > @@ -25,7 +25,6 @@ > #include "amdgpu.h" > #include "amdgpu_ras.h" > #include <linux/bits.h> > -#include "smu_v11_0_i2c.h" > #include "atom.h" > > #define EEPROM_I2C_TARGET_ADDR_VEGA20 0xA0 > @@ -124,6 +123,7 @@ static int __update_table_header(struct amdgpu_ras_eeprom_control *control, > unsigned char *buff) > { > int ret = 0; > + struct amdgpu_device *adev = to_amdgpu_device(control); > struct i2c_msg msg = { > .addr = 0, > .flags = 0, > @@ -137,7 +137,7 @@ static int __update_table_header(struct amdgpu_ras_eeprom_control *control, > > msg.addr = control->i2c_address; > > - ret = i2c_transfer(&control->eeprom_accessor, &msg, 1); > + ret = i2c_transfer(&adev->eeprom_accessor, &msg, 1); > if (ret < 1) > DRM_ERROR("Failed to write EEPROM table header, ret:%d", ret); > > @@ -251,33 +251,18 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control) > .buf = buff, > }; > > + /* Verify i2c adapter is initialized */ > + if (!adev->eeprom_accessor.algo) > + return -ENOENT; > + > if (!__get_eeprom_i2c_addr(adev, &control->i2c_address)) > return -EINVAL; > > mutex_init(&control->tbl_mutex); > > - switch (adev->asic_type) { > - case CHIP_VEGA20: > - ret = smu_v11_0_i2c_eeprom_control_init(&control->eeprom_accessor); > - break; > - > - case CHIP_ARCTURUS: > - ret = smu_i2c_eeprom_init(&adev->smu, &control->eeprom_accessor); > - break; > - > - default: > - return 0; > - } > - > - if (ret) { > - DRM_ERROR("Failed to init I2C controller, ret:%d", ret); > - return ret; > - } > - > msg.addr = control->i2c_address; > - > /* Read/Create table header from EEPROM address 0 */ > - ret = i2c_transfer(&control->eeprom_accessor, &msg, 1); > + ret = i2c_transfer(&adev->eeprom_accessor, &msg, 1); > if (ret < 1) { > DRM_ERROR("Failed to read EEPROM table header, ret:%d", ret); > return ret; > @@ -303,23 +288,6 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control) > return ret == 1 ? 0 : -EIO; > } > > -void amdgpu_ras_eeprom_fini(struct amdgpu_ras_eeprom_control *control) > -{ > - struct amdgpu_device *adev = to_amdgpu_device(control); > - > - switch (adev->asic_type) { > - case CHIP_VEGA20: > - smu_v11_0_i2c_eeprom_control_fini(&control->eeprom_accessor); > - break; > - case CHIP_ARCTURUS: > - smu_i2c_eeprom_fini(&adev->smu, &control->eeprom_accessor); > - break; > - > - default: > - return; > - } > -} > - > static void __encode_table_record_to_buff(struct amdgpu_ras_eeprom_control *control, > struct eeprom_table_record *record, > unsigned char *buff) > @@ -476,7 +444,7 @@ int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, > control->next_addr += EEPROM_TABLE_RECORD_SIZE; > } > > - ret = i2c_transfer(&control->eeprom_accessor, msgs, num); > + ret = i2c_transfer(&adev->eeprom_accessor, msgs, num); > if (ret < 1) { > DRM_ERROR("Failed to process EEPROM table records, ret:%d", ret); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h > index ca78f81..7e8647a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h > @@ -44,7 +44,6 @@ struct amdgpu_ras_eeprom_table_header { > > struct amdgpu_ras_eeprom_control { > struct amdgpu_ras_eeprom_table_header tbl_hdr; > - struct i2c_adapter eeprom_accessor; > uint32_t next_addr; > unsigned int num_recs; > struct mutex tbl_mutex; > @@ -79,7 +78,6 @@ struct eeprom_table_record { > }__attribute__((__packed__)); > > int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control); > -void amdgpu_ras_eeprom_fini(struct amdgpu_ras_eeprom_control *control); > int amdgpu_ras_eeprom_reset_table(struct amdgpu_ras_eeprom_control *control); > > int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, > 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 c902f26..1df57ba 100644 > --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c > +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c > @@ -46,8 +46,7 @@ > #define I2C_NO_STOP 1 > #define I2C_RESTART 2 > > -#define to_amdgpu_device(x) (container_of(x, struct amdgpu_ras, eeprom_control.eeprom_accessor))->adev > -#define to_eeprom_control(x) container_of(x, struct amdgpu_ras_eeprom_control, eeprom_accessor) > +#define to_amdgpu_device(x) (container_of(x, struct amdgpu_device, eeprom_accessor)) > > static void smu_v11_0_i2c_set_clock_gating(struct i2c_adapter *control, bool en) > { > @@ -592,7 +591,8 @@ static uint32_t smu_v11_0_i2c_eeprom_write_data(struct i2c_adapter *control, > > static void lock_bus(struct i2c_adapter *i2c, unsigned int flags) > { > - struct amdgpu_ras_eeprom_control *control = to_eeprom_control(i2c); > + struct amdgpu_device *adev = to_amdgpu_device(i2c); > + struct amdgpu_ras_eeprom_control *control = &adev->psp.ras.ras->eeprom_control; > > if (!smu_v11_0_i2c_bus_lock(i2c)) { > DRM_ERROR("Failed to lock the bus from SMU"); > @@ -610,7 +610,8 @@ static int trylock_bus(struct i2c_adapter *i2c, unsigned int flags) > > static void unlock_bus(struct i2c_adapter *i2c, unsigned int flags) > { > - struct amdgpu_ras_eeprom_control *control = to_eeprom_control(i2c); > + struct amdgpu_device *adev = to_amdgpu_device(i2c); > + struct amdgpu_ras_eeprom_control *control = &adev->psp.ras.ras->eeprom_control; > > if (!smu_v11_0_i2c_bus_unlock(i2c)) { > DRM_ERROR("Failed to unlock the bus from SMU"); > @@ -630,7 +631,8 @@ static int smu_v11_0_i2c_eeprom_i2c_xfer(struct i2c_adapter *i2c_adap, > struct i2c_msg *msgs, int num) > { > int i, ret; > - struct amdgpu_ras_eeprom_control *control = to_eeprom_control(i2c_adap); > + struct amdgpu_device *adev = to_amdgpu_device(i2c_adap); > + struct amdgpu_ras_eeprom_control *control = &adev->psp.ras.ras->eeprom_control; > > if (!control->bus_locked) { > DRM_ERROR("I2C bus unlocked, stopping transaction!"); > @@ -679,7 +681,7 @@ int smu_v11_0_i2c_eeprom_control_init(struct i2c_adapter *control) > control->class = I2C_CLASS_SPD; > control->dev.parent = &adev->pdev->dev; > control->algo = &smu_v11_0_i2c_eeprom_i2c_algo; > - snprintf(control->name, sizeof(control->name), "RAS EEPROM"); > + snprintf(control->name, sizeof(control->name), "AMDGPU EEPROM"); > control->lock_ops = &smu_v11_0_i2c_i2c_lock_ops; > > res = i2c_add_adapter(control); > diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c > index 61596e8..94edb29 100644 > --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c > +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c > @@ -41,7 +41,7 @@ > #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 to_amdgpu_device(x) (container_of(x, struct amdgpu_device, eeprom_accessor)) > > #define CTF_OFFSET_EDGE 5 > #define CTF_OFFSET_HOTSPOT 5 > -- > 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