On Fri, Mar 13, 2020 at 11:22 AM Andrey Grodzovsky
<Andrey.Grodzovsky@xxxxxxx> wrote:
>
>
> On 3/13/20 11:10 AM, Alex Deucher wrote:
> > 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?
>
> amdgpu_pm makes sense for MI-100 where we go through amdgpu_pm interface to send SMU message
> but for MI-60 where we do I2C directly it seems to me at least that this has nothing to with amdgpu_pm
>
Well for vega20, you still have to take the i2c lock in the smu, so we
need to make sure the SMU is initialized first. So I think it makes
sense.
Alex
> Andrey
>
>
> >
> >> };
> >>
> >> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Candrey.grodzovsky%40amd.com%7C5742bd5089f541184ff508d7c760c33e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637197090716969372&sdata=GWCh4fy1nKfoUJm4VvS5iBKIU6i1NzcBjUNF3bs81ms%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
