[PATCH 34/40] drm/amdgpu: Simplify RAS EEPROM checksum calculations
Rename update_table_header() to write_table_header() as this function is actually writing it to EEPROM. Use kernel types; use u8 to carry around the checksum, in order to take advantage of arithmetic modulo 8-bits (256). Tidy up to 80 columns. When updating the checksum, just recalculate the whole thing. Cc: Alexander Deucher Cc: Andrey Grodzovsky Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 98 +-- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h| 2 +- 2 files changed, 50 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index 7d0f9e1e62dc4f..54ef31594accd9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -141,8 +141,8 @@ static void __decode_table_header_from_buff(struct amdgpu_ras_eeprom_table_heade hdr->checksum = le32_to_cpu(pp[4]); } -static int __update_table_header(struct amdgpu_ras_eeprom_control *control, -unsigned char *buff) +static int __write_table_header(struct amdgpu_ras_eeprom_control *control, + unsigned char *buff) { int ret = 0; struct amdgpu_device *adev = to_amdgpu_device(control); @@ -162,69 +162,74 @@ static int __update_table_header(struct amdgpu_ras_eeprom_control *control, return ret; } -static uint32_t __calc_hdr_byte_sum(struct amdgpu_ras_eeprom_control *control) +static u8 __calc_hdr_byte_sum(const struct amdgpu_ras_eeprom_control *control) { int i; - uint32_t tbl_sum = 0; + u8 hdr_sum = 0; + u8 *p; + size_t sz; /* Header checksum, skip checksum field in the calculation */ - for (i = 0; i < sizeof(control->tbl_hdr) - sizeof(control->tbl_hdr.checksum); i++) - tbl_sum += *(((unsigned char *)>tbl_hdr) + i); + sz = sizeof(control->tbl_hdr) - sizeof(control->tbl_hdr.checksum); + p = (u8 *) >tbl_hdr; + for (i = 0; i < sz; i++, p++) + hdr_sum += *p; - return tbl_sum; + return hdr_sum; } -static uint32_t __calc_recs_byte_sum(struct eeprom_table_record *records, - int num) +static u8 __calc_recs_byte_sum(const struct eeprom_table_record *record, + const int num) { int i, j; - uint32_t tbl_sum = 0; + u8 tbl_sum = 0; + + if (!record) + return 0; /* Records checksum */ for (i = 0; i < num; i++) { - struct eeprom_table_record *record = [i]; + u8 *p = (u8 *) [i]; - for (j = 0; j < sizeof(*record); j++) { - tbl_sum += *(((unsigned char *)record) + j); - } + for (j = 0; j < sizeof(*record); j++, p++) + tbl_sum += *p; } return tbl_sum; } -static inline uint32_t __calc_tbl_byte_sum(struct amdgpu_ras_eeprom_control *control, - struct eeprom_table_record *records, int num) +static inline u8 +__calc_tbl_byte_sum(struct amdgpu_ras_eeprom_control *control, + struct eeprom_table_record *records, int num) { - return __calc_hdr_byte_sum(control) + __calc_recs_byte_sum(records, num); + return __calc_hdr_byte_sum(control) + + __calc_recs_byte_sum(records, num); } -/* Checksum = 256 -((sum of all table entries) mod 256) */ static void __update_tbl_checksum(struct amdgpu_ras_eeprom_control *control, - struct eeprom_table_record *records, int num, - uint32_t old_hdr_byte_sum) + struct eeprom_table_record *records, int num) { - /* -* This will update the table sum with new records. -* -* TODO: What happens when the EEPROM table is to be wrapped around -* and old records from start will get overridden. -*/ - - /* need to recalculate updated header byte sum */ - control->tbl_byte_sum -= old_hdr_byte_sum; - control->tbl_byte_sum += __calc_tbl_byte_sum(control, records, num); + u8 v; - control->tbl_hdr.checksum = 256 - (control->tbl_byte_sum % 256); + control->tbl_byte_sum = __calc_tbl_byte_sum(control, records, num); + /* Avoid 32-bit sign extension. */ + v = -control->tbl_byte_sum; + control->tbl_hdr.checksum = v; } -/* table sum mod 256 + checksum must equals 256 */ -static bool __validate_tbl_checksum(struct amdgpu_ras_eeprom_control *control, - struct eeprom_table_record *records, int num) +static bool __verify_tbl_checksum(struct amdgpu_ras_eeprom_control *control, + struct eeprom_table_record *records, + int num)
[PATCH 37/40] drm/amdgpu: RAS EEPROM table is now in debugfs
Add "ras_eeprom_size" file in debugfs, which reports the maximum size allocated to the RAS table in EEROM, as the number of bytes and the number of records it could store. For instance, $cat /sys/kernel/debug/dri/0/ras/ras_eeprom_size 262144 bytes or 10921 records $_ Add "ras_eeprom_table" file in debugfs, which dumps the RAS table stored EEPROM, in a formatted way. For instance, $cat ras_eeprom_table SignatureVersion FirstOffs Size Checksum 0x414D4452 0x0001 0x0014 0x00EC 0x00DA Index Offset ErrType Bank/CU TimeStamp Offs/Addr MemChl MCUMCID RetiredPage 0 0x00014 ue0x00 0x607608DC 0x 0x000x00 0x 1 0x0002C ue0x00 0x607608DC 0x1000 0x000x00 0x0001 2 0x00044 ue0x00 0x607608DC 0x2000 0x000x00 0x0002 3 0x0005C ue0x00 0x607608DC 0x3000 0x000x00 0x0003 4 0x00074 ue0x00 0x607608DC 0x4000 0x000x00 0x0004 5 0x0008C ue0x00 0x607608DC 0x5000 0x000x00 0x0005 6 0x000A4 ue0x00 0x607608DC 0x6000 0x000x00 0x0006 7 0x000BC ue0x00 0x607608DC 0x7000 0x000x00 0x0007 8 0x000D4 ue0x00 0x607608DD 0x8000 0x000x00 0x0008 $_ Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: John Clements Cc: Hawking Zhang Cc: Xinhui Pan Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 12 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 1 + .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 241 +- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h| 10 +- 4 files changed, 252 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 37e52cf0ce1d92..28378c7e729608 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -404,9 +404,9 @@ static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f, /* umc ce/ue error injection for a bad page is not allowed */ if ((data.head.block == AMDGPU_RAS_BLOCK__UMC) && amdgpu_ras_check_bad_page(adev, data.inject.address)) { - dev_warn(adev->dev, "RAS WARN: 0x%llx has been marked " - "as bad before error injection!\n", - data.inject.address); + dev_warn(adev->dev, "RAS WARN: inject: 0x%llx has " +"already been marked as bad!\n", +data.inject.address); break; } @@ -1301,6 +1301,12 @@ static struct dentry *amdgpu_ras_debugfs_create_ctrl_node(struct amdgpu_device * >bad_page_cnt_threshold); debugfs_create_x32("ras_hw_enabled", 0444, dir, >ras_hw_enabled); debugfs_create_x32("ras_enabled", 0444, dir, >ras_enabled); + debugfs_create_file("ras_eeprom_size", S_IRUGO, dir, adev, + _ras_debugfs_eeprom_size_ops); + con->de_ras_eeprom_table = debugfs_create_file("ras_eeprom_table", + S_IRUGO, dir, adev, + _ras_debugfs_eeprom_table_ops); + amdgpu_ras_debugfs_set_ret_size(>eeprom_control); /* * After one uncorrectable error happens, usually GPU recovery will diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h index 256cea5d34f2b6..283afd791db107 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h @@ -318,6 +318,7 @@ struct amdgpu_ras { /* sysfs */ struct device_attribute features_attr; struct bin_attribute badpages_attr; + struct dentry *de_ras_eeprom_table; /* block array */ struct ras_manager *objs; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index dc4a845a32404c..677e379f5fb5e9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -27,6 +27,8 @@ #include #include "atom.h" #include "amdgpu_eeprom.h" +#include +#include #define EEPROM_I2C_MADDR_VEGA20 0x0 #define EEPROM_I2C_MADDR_ARCTURUS 0x4 @@ -70,6 +72,13 @@ #define RAS_OFFSET_TO_INDEX(_C, _O) (((_O) - \ (_C)->ras_record_offset) / RAS_TABLE_RECORD_SIZE) +/* Given a 0-based relative record index, 0, 1, 2, ..., etc., off + * of "fri", return the absolute record index off of the end of + * the table header. + */
[PATCH 31/40] drm/amdgpu: Fix width of I2C address
The I2C address is kept as a 16-bit quantity in the kernel. The I2C_TAR::I2C_TAR field is 10-bit wide. Fix the width of the I2C address for Vega20 from 8 bits to 16 bits to accommodate the full spectrum of I2C address space. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) 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 e403ba556e5590..65035256756679 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -111,12 +111,15 @@ static void smu_v11_0_i2c_set_clock(struct i2c_adapter *control) WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_SDA_HOLD, 20); } -static void smu_v11_0_i2c_set_address(struct i2c_adapter *control, uint8_t address) +static void smu_v11_0_i2c_set_address(struct i2c_adapter *control, u16 address) { struct amdgpu_device *adev = to_amdgpu_device(control); - /* We take 7-bit addresses raw */ - WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_TAR, (address & 0xFF)); + /* The IC_TAR::IC_TAR field is 10-bits wide. +* It takes a 7-bit or 10-bit addresses as an address, +* i.e. no read/write bit--no wire format, just the address. +*/ + WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_TAR, address & 0x3FF); } static uint32_t smu_v11_0_i2c_poll_tx_status(struct i2c_adapter *control) @@ -215,8 +218,8 @@ static uint32_t smu_v11_0_i2c_poll_rx_status(struct i2c_adapter *control) * Returns 0 on success or error. */ static uint32_t smu_v11_0_i2c_transmit(struct i2c_adapter *control, - uint8_t address, uint8_t *data, - uint32_t numbytes, uint32_t i2c_flag) + u16 address, u8 *data, + u32 numbytes, u32 i2c_flag) { struct amdgpu_device *adev = to_amdgpu_device(control); uint32_t bytes_sent, reg, ret = 0; @@ -225,7 +228,7 @@ static uint32_t smu_v11_0_i2c_transmit(struct i2c_adapter *control, bytes_sent = 0; DRM_DEBUG_DRIVER("I2C_Transmit(), address = %x, bytes = %d , data: ", -(uint16_t)address, numbytes); +address, numbytes); if (drm_debug_enabled(DRM_UT_DRIVER)) { print_hex_dump(KERN_INFO, "data: ", DUMP_PREFIX_NONE, @@ -318,8 +321,8 @@ static uint32_t smu_v11_0_i2c_transmit(struct i2c_adapter *control, * Returns 0 on success or error. */ static uint32_t smu_v11_0_i2c_receive(struct i2c_adapter *control, -uint8_t address, uint8_t *data, -uint32_t numbytes, uint8_t i2c_flag) + u16 address, u8 *data, + u32 numbytes, u32 i2c_flag) { struct amdgpu_device *adev = to_amdgpu_device(control); uint32_t bytes_received, ret = I2C_OK; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 38/40] drm/amdgpu: Fix koops when accessing RAS EEPROM
Debugfs RAS EEPROM files are available when the ASIC supports RAS, and when the debugfs is enabled, an also when "ras_enable" module parameter is set to 0. However in this case, we get a kernel oops when accessing some of the "ras_..." controls in debugfs. The reason for this is that struct amdgpu_ras::adev is unset. This commit sets it, thus enabling access to those facilities. Note that this facilitates EEPROM access and not necessarily RAS features or functionality. Cc: Alexander Deucher Cc: John Clements Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 28378c7e729608..384031ec82f4f8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1947,11 +1947,20 @@ int amdgpu_ras_recovery_init(struct amdgpu_device *adev) bool exc_err_limit = false; int ret; - if (adev->ras_enabled && con) - data = >eh_data; - else + if (!con) + return 0; + + /* Allow access to RAS EEPROM via debugfs, when the ASIC +* supports RAS and debugfs is enabled, but when +* adev->ras_enabled is unset, i.e. when "ras_enable" +* module parameter is set to 0. +*/ + con->adev = adev; + + if (!adev->ras_enabled) return 0; + data = >eh_data; *data = kmalloc(sizeof(**data), GFP_KERNEL | __GFP_ZERO); if (!*data) { ret = -ENOMEM; @@ -1961,7 +1970,6 @@ int amdgpu_ras_recovery_init(struct amdgpu_device *adev) mutex_init(>recovery_lock); INIT_WORK(>recovery_work, amdgpu_ras_do_recovery); atomic_set(>in_recovery, 0); - con->adev = adev; max_eeprom_records_count = amdgpu_ras_eeprom_max_record_count(); amdgpu_ras_validate_threshold(adev, max_eeprom_records_count); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 40/40] drm/amdgpu: Correctly disable the I2C IP block
On long transfers to the EEPROM device, i.e. write, it is observed that the driver aborts the transfer. The reason for this is that the driver isn't patient enough--the IC_STATUS register's contents is 0x27, which is MST_ACTIVITY | TFE | TFNF | ACTIVITY. That is, while the transmission FIFO is empty, we, the I2C master device, are still driving the bus. Implement the correct procedure to disable the block, as described in the DesignWare I2C Databook, section 3.8.3 Disabling DW_apb_i2c on page 56. Now there are no premature aborts on long data transfers. Cc: Alexander Deucher Cc: Andrey Grodzovsky Signed-off-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 80 +- 1 file changed, 62 insertions(+), 18 deletions(-) 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 751ea2517c4380..7d74d6204d8d0a 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -54,12 +54,48 @@ static void smu_v11_0_i2c_set_clock_gating(struct i2c_adapter *control, bool en) WREG32_SOC15(SMUIO, 0, mmSMUIO_PWRMGT, reg); } +/* The T_I2C_POLL_US is defined as follows: + * + * "Define a timer interval (t_i2c_poll) equal to 10 times the + * signalling period for the highest I2C transfer speed used in the + * system and supported by DW_apb_i2c. For instance, if the highest + * I2C data transfer mode is 400 kb/s, then t_i2c_poll is 25 us." -- + * DesignWare DW_apb_i2c Databook, Version 1.21a, section 3.8.3.1, + * page 56, with grammar and syntax corrections. + * + * Vcc for our device is at 1.8V which puts it at 400 kHz, + * see Atmel AT24CM02 datasheet, section 8.3 DC Characteristics table, page 14. + * + * The procedure to disable the IP block is described in section + * 3.8.3 Disabling DW_apb_i2c on page 56. + */ +#define I2C_SPEED_MODE_FAST 2 +#define T_I2C_POLL_US 25 +#define I2C_MAX_T_POLL_COUNT1000 -static void smu_v11_0_i2c_enable(struct i2c_adapter *control, bool enable) +static int smu_v11_0_i2c_enable(struct i2c_adapter *control, bool enable) { struct amdgpu_device *adev = to_amdgpu_device(control); WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE, enable ? 1 : 0); + + if (!enable) { + int ii; + + for (ii = I2C_MAX_T_POLL_COUNT; ii > 0; ii--) { + u32 en_stat = RREG32_SOC15(SMUIO, + 0, + mmCKSVII2C_IC_ENABLE_STATUS); + if (REG_GET_FIELD(en_stat, CKSVII2C_IC_ENABLE_STATUS, IC_EN)) + udelay(T_I2C_POLL_US); + else + return I2C_OK; + } + + return I2C_ABORT; + } + + return I2C_OK; } static void smu_v11_0_i2c_clear_status(struct i2c_adapter *control) @@ -81,8 +117,13 @@ static void smu_v11_0_i2c_configure(struct i2c_adapter *control) reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_RESTART_EN, 1); reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_10BITADDR_MASTER, 0); reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_10BITADDR_SLAVE, 0); - /* Standard mode */ - reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MAX_SPEED_MODE, 2); + /* The values of IC_MAX_SPEED_MODE are, +* 1: standard mode, 0 - 100 Kb/s, +* 2: fast mode, <= 400 Kb/s, or fast mode plus, <= 1000 Kb/s, +* 3: high speed mode, <= 3.4 Mb/s. +*/ + reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MAX_SPEED_MODE, + I2C_SPEED_MODE_FAST); reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MASTER_MODE, 1); WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_CON, reg); @@ -404,7 +445,6 @@ static void smu_v11_0_i2c_abort(struct i2c_adapter *control) DRM_DEBUG_DRIVER("I2C_Abort() Done."); } - static bool smu_v11_0_i2c_activity_done(struct i2c_adapter *control) { struct amdgpu_device *adev = to_amdgpu_device(control); @@ -416,7 +456,6 @@ static bool smu_v11_0_i2c_activity_done(struct i2c_adapter *control) reg_ic_enable_status = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE_STATUS); reg_ic_enable = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE); - if ((REG_GET_FIELD(reg_ic_enable, CKSVII2C_IC_ENABLE, ENABLE) == 0) && (REG_GET_FIELD(reg_ic_enable_status, CKSVII2C_IC_ENABLE_STATUS, IC_EN) == 1)) { /* @@ -446,6 +485,8 @@ static bool smu_v11_0_i2c_activity_done(struct i2c_adapter *control) static void smu_v11_0_i2c_init(struct i2c_adapter *control) { + int res; + /* Disable clock gating */ smu_v11_0_i2c_set_clock_gating(control, false); @@ -453,7 +494,9 @@ static void smu_v11_0_i2c_init(struct i2c_adapter *control) DRM_WARN("I2C busy !"); /* Disable I2C */ -
[PATCH 39/40] drm/amdgpu: Use a single loop
In smu_v11_0_i2c_transmit() use a single loop to transmit bytes, instead of two nested loops. Cc: Alexander Deucher Cc: Andrey Grodzovsky Signed-off-by: Luben Tuikov Reviewed-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 72 ++ 1 file changed, 34 insertions(+), 38 deletions(-) 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 7f48ee020bc03e..751ea2517c4380 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -243,49 +243,45 @@ static uint32_t smu_v11_0_i2c_transmit(struct i2c_adapter *control, /* Clear status bits */ smu_v11_0_i2c_clear_status(control); - timeout_counter = jiffies + msecs_to_jiffies(20); while (numbytes > 0) { reg = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_STATUS); - if (REG_GET_FIELD(reg, CKSVII2C_IC_STATUS, TFNF)) { - do { - reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, DAT, data[bytes_sent]); - - /* Final message, final byte, must -* generate a STOP, to release the -* bus, i.e. don't hold SCL low. -*/ - if (numbytes == 1 && i2c_flag & I2C_M_STOP) - reg = REG_SET_FIELD(reg, - CKSVII2C_IC_DATA_CMD, - STOP, 1); - - if (bytes_sent == 0 && i2c_flag & I2C_X_RESTART) - reg = REG_SET_FIELD(reg, - CKSVII2C_IC_DATA_CMD, - RESTART, 1); - - /* Write */ - reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, CMD, 0); - WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_DATA_CMD, reg); - - /* Record that the bytes were transmitted */ - bytes_sent++; - numbytes--; - - reg = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_STATUS); - - } while (numbytes && REG_GET_FIELD(reg, CKSVII2C_IC_STATUS, TFNF)); - } + if (!REG_GET_FIELD(reg, CKSVII2C_IC_STATUS, TFNF)) { + /* +* We waited for too long for the transmission +* FIFO to become not-full. Exit the loop +* with error. +*/ + if (time_after(jiffies, timeout_counter)) { + ret |= I2C_SW_TIMEOUT; + goto Err; + } + } else { + reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, DAT, + data[bytes_sent]); - /* -* We waited too long for the transmission FIFO to become not-full. -* Exit the loop with error. -*/ - if (time_after(jiffies, timeout_counter)) { - ret |= I2C_SW_TIMEOUT; - goto Err; + /* Final message, final byte, must generate a +* STOP to release the bus, i.e. don't hold +* SCL low. +*/ + if (numbytes == 1 && i2c_flag & I2C_M_STOP) + reg = REG_SET_FIELD(reg, + CKSVII2C_IC_DATA_CMD, + STOP, 1); + + if (bytes_sent == 0 && i2c_flag & I2C_X_RESTART) + reg = REG_SET_FIELD(reg, + CKSVII2C_IC_DATA_CMD, + RESTART, 1); + + /* Write */ + reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, CMD, 0); + WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_DATA_CMD, reg); + + /* Record that the bytes were transmitted */ + bytes_sent++; + numbytes--; } } -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 33/40] drm/amdgpu: Fix amdgpu_ras_eeprom_init()
No need to account for the 2 bytes of EEPROM address--this is now well abstracted away by the fixes the the lower layers. Cc: Andrey Grodzovsky Cc: Alexander Deucher Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index dc48c556398039..7d0f9e1e62dc4f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -306,7 +306,7 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control, return ret; } - __decode_table_header_from_buff(hdr, [2]); + __decode_table_header_from_buff(hdr, buff); if (hdr->header == RAS_TABLE_HDR_VAL) { control->num_recs = (hdr->tbl_size - RAS_TABLE_HEADER_SIZE) / -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 22/40] drm/amdgpu: RAS and FRU now use 19-bit I2C address
Convert RAS and FRU code to use the 19-bit I2C memory address and remove all "slave_addr", as this is now absolved into the 19-bit address. Cc: Jean Delvare Cc: John Clements Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c| 19 ++--- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 82 +++ .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h| 2 +- 3 files changed, 39 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c index 2b854bc6ae34bb..69b9559f840ac3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c @@ -29,8 +29,8 @@ #include "amdgpu_fru_eeprom.h" #include "amdgpu_eeprom.h" -#define I2C_PRODUCT_INFO_ADDR 0x56 -#define I2C_PRODUCT_INFO_OFFSET0xC0 +#define FRU_EEPROM_MADDR0x6 +#define I2C_PRODUCT_INFO_OFFSET 0xC0 static bool is_fru_eeprom_supported(struct amdgpu_device *adev) { @@ -62,12 +62,11 @@ static bool is_fru_eeprom_supported(struct amdgpu_device *adev) } static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr, - unsigned char *buff) + unsigned char *buff) { int ret, size; - ret = amdgpu_eeprom_xfer(>pm.smu_i2c, I2C_PRODUCT_INFO_ADDR, -addrptr, buff, 1, true); + ret = amdgpu_eeprom_xfer(>pm.smu_i2c, addrptr, buff, 1, true); if (ret < 1) { DRM_WARN("FRU: Failed to get size field"); return ret; @@ -78,8 +77,8 @@ static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr, */ size = buff[0] - I2C_PRODUCT_INFO_OFFSET; - ret = amdgpu_eeprom_xfer(>pm.smu_i2c, I2C_PRODUCT_INFO_ADDR, -addrptr + 1, buff, size, true); + ret = amdgpu_eeprom_xfer(>pm.smu_i2c, addrptr + 1, buff, size, +true); if (ret < 1) { DRM_WARN("FRU: Failed to get data field"); return ret; @@ -91,8 +90,8 @@ static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr, int amdgpu_fru_get_product_info(struct amdgpu_device *adev) { unsigned char buff[34]; - int addrptr, size; - int len; + u32 addrptr; + int size, len; if (!is_fru_eeprom_supported(adev)) return 0; @@ -115,7 +114,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev) * Bytes 8-a are all 1-byte and refer to the size of the entire struct, * and the language field, so just start from 0xb, manufacturer size */ - addrptr = 0xb; + addrptr = FRU_EEPROM_MADDR + 0xb; size = amdgpu_fru_read_eeprom(adev, addrptr, buff); if (size < 1) { DRM_ERROR("Failed to read FRU Manufacturer, ret:%d", size); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index 2b981e96ce5b9e..f316fb11b16d9e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -28,11 +28,11 @@ #include "atom.h" #include "amdgpu_eeprom.h" -#define EEPROM_I2C_TARGET_ADDR_VEGA20 0x50 -#define EEPROM_I2C_TARGET_ADDR_ARCTURUS0x54 -#define EEPROM_I2C_TARGET_ADDR_ARCTURUS_D342 0x50 -#define EEPROM_I2C_TARGET_ADDR_SIENNA_CICHLID 0x50 -#define EEPROM_I2C_TARGET_ADDR_ALDEBARAN0x50 +#define EEPROM_I2C_MADDR_VEGA20 0x0 +#define EEPROM_I2C_MADDR_ARCTURUS 0x4 +#define EEPROM_I2C_MADDR_ARCTURUS_D342 0x0 +#define EEPROM_I2C_MADDR_SIENNA_CICHLID 0x0 +#define EEPROM_I2C_MADDR_ALDEBARAN 0x0 /* * The 2 macros bellow represent the actual size in bytes that @@ -58,7 +58,6 @@ #define EEPROM_HDR_START 0 #define EEPROM_RECORD_START (EEPROM_HDR_START + EEPROM_TABLE_HEADER_SIZE) #define EEPROM_MAX_RECORD_NUM ((EEPROM_SIZE_BYTES - EEPROM_TABLE_HEADER_SIZE) / EEPROM_TABLE_RECORD_SIZE) -#define EEPROM_ADDR_MSB_MASK GENMASK(17, 8) #define to_amdgpu_device(x) (container_of(x, struct amdgpu_ras, eeprom_control))->adev @@ -74,43 +73,43 @@ static bool __is_ras_eeprom_supported(struct amdgpu_device *adev) } static bool __get_eeprom_i2c_addr_arct(struct amdgpu_device *adev, - uint16_t *i2c_addr) + struct amdgpu_ras_eeprom_control *control) { struct atom_context *atom_ctx = adev->mode_info.atom_context; - if (!i2c_addr || !atom_ctx) + if (!control || !atom_ctx) return false; if (strnstr(atom_ctx->vbios_version, "D342",
[PATCH 36/40] drm/amdgpu: Optimizations to EEPROM RAS table I/O
Read and write the table in one go, then using a separate stage to decode or encode the data and reading/writing the table, as opposed to on the fly, which keeps the I2C bus busy. Use a single read/write to read/write the table or at most two if the number of records we're reading/writing wraps around. Check the check-sum of a table in EEPROM on init. When updating the table header signature, when the threshold was increased on boot, also update the check-sum at that time. Split functionality between read and write, which simplifies the code and exposes areas of optimization and complexity. Cc: Alexander Deucher Cc: Andrey Grodzovsky Signed-off-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c| 20 +- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h| 23 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 20 +- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 903 +++--- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h| 53 +- 5 files changed, 655 insertions(+), 364 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c index a4815af111ed12..4c3c65a5acae9b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -176,8 +176,8 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, * * Returns the number of bytes read/written; -errno on error. */ -int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, - u8 *eeprom_buf, u16 buf_size, bool read) +static int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, + u8 *eeprom_buf, u16 buf_size, bool read) { const struct i2c_adapter_quirks *quirks = i2c_adap->quirks; u16 limit; @@ -221,3 +221,19 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, return res; } } + +int amdgpu_eeprom_read(struct i2c_adapter *i2c_adap, + u32 eeprom_addr, u8 *eeprom_buf, + u16 bytes) +{ + return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes, + true); +} + +int amdgpu_eeprom_write(struct i2c_adapter *i2c_adap, + u32 eeprom_addr, u8 *eeprom_buf, + u16 bytes) +{ + return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes, + false); +} diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h index 966b434f0de2b7..6935adb2be1f1c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h @@ -26,23 +26,12 @@ #include -int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, - u8 *eeprom_buf, u16 bytes, bool read); +int amdgpu_eeprom_read(struct i2c_adapter *i2c_adap, + u32 eeprom_addr, u8 *eeprom_buf, + u16 bytes); -static inline int amdgpu_eeprom_read(struct i2c_adapter *i2c_adap, -u32 eeprom_addr, u8 *eeprom_buf, -u16 bytes) -{ - return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes, - true); -} - -static inline int amdgpu_eeprom_write(struct i2c_adapter *i2c_adap, - u32 eeprom_addr, u8 *eeprom_buf, - u16 bytes) -{ - return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes, - false); -} +int amdgpu_eeprom_write(struct i2c_adapter *i2c_adap, + u32 eeprom_addr, u8 *eeprom_buf, + u16 bytes); #endif diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index ecdf2d80eee8f0..37e52cf0ce1d92 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -451,7 +451,7 @@ static ssize_t amdgpu_ras_debugfs_eeprom_write(struct file *f, ret = amdgpu_ras_eeprom_reset_table( &(amdgpu_ras_get_context(adev)->eeprom_control)); - if (ret > 0) { + if (!ret) { /* Something was written to EEPROM. */ amdgpu_ras_get_context(adev)->flags = RAS_DEFAULT_FLAGS; @@ -1818,12 +1818,12 @@ int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev) control = >eeprom_control; data = con->eh_data; - save_count = data->count - control->num_recs; + save_count = data->count - control->ras_num_recs; /* only new entries are saved */ if (save_count > 0) { - if (amdgpu_ras_eeprom_write(control, - >bps[control->num_recs], - save_count)) { + if
[PATCH 27/40] drm/amdgpu: RAS xfer to read/write
Wrap amdgpu_ras_eeprom_xfer(..., bool write), into amdgpu_ras_eeprom_read() and amdgpu_ras_eeprom_write(), as that makes reading and understanding the code clearer. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 9 --- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 24 +++ .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h| 8 --- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index edd4d83e1e392f..9a878b5a7665fb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1817,10 +1817,9 @@ int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev) save_count = data->count - control->num_recs; /* only new entries are saved */ if (save_count > 0) { - if (amdgpu_ras_eeprom_xfer(control, - >bps[control->num_recs], - save_count, - true)) { + if (amdgpu_ras_eeprom_write(control, + >bps[control->num_recs], + save_count)) { dev_err(adev->dev, "Failed to save EEPROM table data!"); return -EIO; } @@ -1850,7 +1849,7 @@ static int amdgpu_ras_load_bad_pages(struct amdgpu_device *adev) if (!bps) return -ENOMEM; - if (amdgpu_ras_eeprom_xfer(control, bps, control->num_recs, false)) { + if (amdgpu_ras_eeprom_read(control, bps, control->num_recs)) { dev_err(adev->dev, "Failed to load EEPROM table records!"); ret = -EIO; goto out; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index 9e3fbc44b4bc4a..550a31953d2da1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -432,9 +432,9 @@ bool amdgpu_ras_eeprom_check_err_threshold(struct amdgpu_device *adev) return false; } -int amdgpu_ras_eeprom_xfer(struct amdgpu_ras_eeprom_control *control, - struct eeprom_table_record *records, - const u32 num, bool write) +static int amdgpu_ras_eeprom_xfer(struct amdgpu_ras_eeprom_control *control, + struct eeprom_table_record *records, + const u32 num, bool write) { int i, ret = 0; unsigned char *buffs, *buff; @@ -554,6 +554,20 @@ int amdgpu_ras_eeprom_xfer(struct amdgpu_ras_eeprom_control *control, return ret == num ? 0 : -EIO; } +int amdgpu_ras_eeprom_read(struct amdgpu_ras_eeprom_control *control, + struct eeprom_table_record *records, + const u32 num) +{ + return amdgpu_ras_eeprom_xfer(control, records, num, false); +} + +int amdgpu_ras_eeprom_write(struct amdgpu_ras_eeprom_control *control, + struct eeprom_table_record *records, + const u32 num) +{ + return amdgpu_ras_eeprom_xfer(control, records, num, true); +} + inline uint32_t amdgpu_ras_eeprom_get_record_max_length(void) { return RAS_MAX_RECORD_NUM; @@ -574,13 +588,13 @@ void amdgpu_ras_eeprom_test(struct amdgpu_ras_eeprom_control *control) recs[i].retired_page = i; } - if (!amdgpu_ras_eeprom_xfer(control, recs, 1, true)) { + if (!amdgpu_ras_eeprom_write(control, recs, 1)) { memset(recs, 0, sizeof(*recs) * 1); control->next_addr = RAS_RECORD_START; - if (!amdgpu_ras_eeprom_xfer(control, recs, 1, false)) { + if (!amdgpu_ras_eeprom_read(control, recs)) { for (i = 0; i < 1; i++) DRM_INFO("rec.address :0x%llx, rec.retired_page :%llu", recs[i].address, recs[i].retired_page); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h index 6a1bd527bce57a..fa9c509a8e2f2b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h @@ -82,9 +82,11 @@ int amdgpu_ras_eeprom_reset_table(struct amdgpu_ras_eeprom_control *control); bool amdgpu_ras_eeprom_check_err_threshold(struct amdgpu_device *adev); -int amdgpu_ras_eeprom_xfer(struct amdgpu_ras_eeprom_control *control, - struct eeprom_table_record *records, - const u32 num, bool write); +int amdgpu_ras_eeprom_read(struct amdgpu_ras_eeprom_control
[PATCH 25/40] drm/amdgpu: RAS: EEPROM --> RAS
In amdgpu_ras_eeprom.c--the interface from RAS to EEPROM, rename macros from EEPROM to RAS, to indicate that the quantities and objects are RAS specific, not EEPROM. We can decrease the RAS table, or put it in different offset of EEPROM as needed in the future. Remove EEPROM_ADDRESS_SIZE macro definition, equal to 2, from the file and calculations, as that quantity is computed and added on the stack, in the lower layer, amdgpu_eeprom_xfer(). Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 103 +- 1 file changed, 50 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index 3ef38b90fc3a83..d3678706bb736d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -37,26 +37,25 @@ /* * The 2 macros bellow represent the actual size in bytes that * those entities occupy in the EEPROM memory. - * EEPROM_TABLE_RECORD_SIZE is different than sizeof(eeprom_table_record) which + * RAS_TABLE_RECORD_SIZE is different than sizeof(eeprom_table_record) which * uses uint64 to store 6b fields such as retired_page. */ -#define EEPROM_TABLE_HEADER_SIZE 20 -#define EEPROM_TABLE_RECORD_SIZE 24 - -#define EEPROM_ADDRESS_SIZE 0x2 +#define RAS_TABLE_HEADER_SIZE 20 +#define RAS_TABLE_RECORD_SIZE 24 /* Table hdr is 'AMDR' */ -#define EEPROM_TABLE_HDR_VAL 0x414d4452 -#define EEPROM_TABLE_VER 0x0001 +#define RAS_TABLE_HDR_VAL 0x414d4452 +#define RAS_TABLE_VER 0x0001 /* Bad GPU tag ‘BADG’ */ -#define EEPROM_TABLE_HDR_BAD 0x42414447 +#define RAS_TABLE_HDR_BAD 0x42414447 -/* Assume 2-Mbit size */ -#define EEPROM_SIZE_BYTES (256 * 1024) -#define EEPROM_HDR_START0 -#define EEPROM_RECORD_START (EEPROM_HDR_START + EEPROM_TABLE_HEADER_SIZE) -#define EEPROM_MAX_RECORD_NUM ((EEPROM_SIZE_BYTES - EEPROM_TABLE_HEADER_SIZE) / EEPROM_TABLE_RECORD_SIZE) +/* Assume 2-Mbit size EEPROM and take up the whole space. */ +#define RAS_TBL_SIZE_BYTES (256 * 1024) +#define RAS_HDR_START 0 +#define RAS_RECORD_START(RAS_HDR_START + RAS_TABLE_HEADER_SIZE) +#define RAS_MAX_RECORD_NUM ((RAS_TBL_SIZE_BYTES - RAS_TABLE_HEADER_SIZE) \ +/ RAS_TABLE_RECORD_SIZE) #define to_amdgpu_device(x) (container_of(x, struct amdgpu_ras, eeprom_control))->adev @@ -153,8 +152,8 @@ static int __update_table_header(struct amdgpu_ras_eeprom_control *control, /* i2c may be unstable in gpu reset */ down_read(>reset_sem); ret = amdgpu_eeprom_xfer(>pm.smu_i2c, -control->i2c_address + EEPROM_HDR_START, -buff, EEPROM_TABLE_HEADER_SIZE, false); +control->i2c_address + RAS_HDR_START, +buff, RAS_TABLE_HEADER_SIZE, false); up_read(>reset_sem); if (ret < 1) @@ -236,11 +235,11 @@ static int amdgpu_ras_eeprom_correct_header_tag( struct amdgpu_ras_eeprom_control *control, uint32_t header) { - unsigned char buff[EEPROM_ADDRESS_SIZE + EEPROM_TABLE_HEADER_SIZE]; + unsigned char buff[RAS_TABLE_HEADER_SIZE]; struct amdgpu_ras_eeprom_table_header *hdr = >tbl_hdr; int ret = 0; - memset(buff, 0, EEPROM_ADDRESS_SIZE + EEPROM_TABLE_HEADER_SIZE); + memset(buff, 0, RAS_TABLE_HEADER_SIZE); mutex_lock(>tbl_mutex); hdr->header = header; @@ -252,20 +251,20 @@ static int amdgpu_ras_eeprom_correct_header_tag( int amdgpu_ras_eeprom_reset_table(struct amdgpu_ras_eeprom_control *control) { - unsigned char buff[EEPROM_ADDRESS_SIZE + EEPROM_TABLE_HEADER_SIZE] = { 0 }; + unsigned char buff[RAS_TABLE_HEADER_SIZE] = { 0 }; struct amdgpu_ras_eeprom_table_header *hdr = >tbl_hdr; int ret = 0; mutex_lock(>tbl_mutex); - hdr->header = EEPROM_TABLE_HDR_VAL; - hdr->version = EEPROM_TABLE_VER; - hdr->first_rec_offset = EEPROM_RECORD_START; - hdr->tbl_size = EEPROM_TABLE_HEADER_SIZE; + hdr->header = RAS_TABLE_HDR_VAL; + hdr->version = RAS_TABLE_VER; + hdr->first_rec_offset = RAS_RECORD_START; + hdr->tbl_size = RAS_TABLE_HEADER_SIZE; control->tbl_byte_sum = 0; __update_tbl_checksum(control, NULL, 0, 0); - control->next_addr = EEPROM_RECORD_START; + control->next_addr = RAS_RECORD_START; ret = __update_table_header(control, buff); @@ -280,7 +279,7 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control, { int ret = 0; struct amdgpu_device *adev = to_amdgpu_device(control); - unsigned char
[PATCH 19/40] drm/amdgpu: Fixes to the AMDGPU EEPROM driver
* When reading from the EEPROM device, there is no device limitation on the number of bytes read--they're simply sequenced out. Thus, read the whole data requested in one go. * When writing to the EEPROM device, there is a 256-byte page limit to write to before having to generate a STOP on the bus, as well as the address written to mustn't cross over the page boundary (it actually rolls over). Maximize the data written to per bus acquisition. * Return the number of bytes read/written, or -errno. * Add kernel doc. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 96 +++--- 1 file changed, 68 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c index d02ea083a6c69b..7fdb5bd2fc8bc8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -24,59 +24,99 @@ #include "amdgpu_eeprom.h" #include "amdgpu.h" -#define EEPROM_OFFSET_LENGTH 2 +/* AT24CM02 has a 256-byte write page size. + */ +#define EEPROM_PAGE_BITS 8 +#define EEPROM_PAGE_SIZE (1U << EEPROM_PAGE_BITS) +#define EEPROM_PAGE_MASK (EEPROM_PAGE_SIZE - 1) + +#define EEPROM_OFFSET_SIZE 2 +/** + * amdgpu_eeprom_xfer -- Read/write from/to an I2C EEPROM device + * @i2c_adap: pointer to the I2C adapter to use + * @slave_addr: I2C address of the slave device + * @eeprom_addr: EEPROM address from which to read/write + * @eeprom_buf: pointer to data buffer to read into/write from + * @buf_size: the size of @eeprom_buf + * @read: True if reading from the EEPROM, false if writing + * + * Returns the number of bytes read/written; -errno on error. + */ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u16 slave_addr, u16 eeprom_addr, - u8 *eeprom_buf, u16 bytes, bool read) + u8 *eeprom_buf, u16 buf_size, bool read) { - u8 eeprom_offset_buf[2]; - u16 bytes_transferred; + u8 eeprom_offset_buf[EEPROM_OFFSET_SIZE]; struct i2c_msg msgs[] = { { .addr = slave_addr, .flags = 0, - .len = EEPROM_OFFSET_LENGTH, + .len = EEPROM_OFFSET_SIZE, .buf = eeprom_offset_buf, }, { .addr = slave_addr, .flags = read ? I2C_M_RD : 0, - .len = bytes, - .buf = eeprom_buf, }, }; + const u8 *p = eeprom_buf; int r; + u16 len; + + r = 0; + for (len = 0; buf_size > 0; +buf_size -= len, eeprom_addr += len, eeprom_buf += len) { + /* Set the EEPROM address we want to write to/read from. +*/ + msgs[0].buf[0] = (eeprom_addr >> 8) & 0xff; + msgs[0].buf[1] = eeprom_addr & 0xff; - msgs[0].buf[0] = ((eeprom_addr >> 8) & 0xff); - msgs[0].buf[1] = (eeprom_addr & 0xff); + if (!read) { + /* Write the maximum amount of data, without +* crossing the device's page boundary, as per +* its spec. Partial page writes are allowed, +* starting at any location within the page, +* so long as the page boundary isn't crossed +* over (actually the page pointer rolls +* over). +* +* As per the AT24CM02 EEPROM spec, after +* writing into a page, the I2C driver MUST +* terminate the transfer, i.e. in +* "i2c_transfer()" below, with a STOP +* condition, so that the self-timed write +* cycle begins. This is implied for the +* "i2c_transfer()" abstraction. +*/ + len = min(EEPROM_PAGE_SIZE - (eeprom_addr & + EEPROM_PAGE_MASK), + (u32)buf_size); + } else { + /* Reading from the EEPROM has no limitation +* on the number of bytes read from the EEPROM +* device--they are simply sequenced out. +*/ + len = buf_size; + } + msgs[1].len = len; + msgs[1].buf = eeprom_buf; - while (msgs[1].len) { r = i2c_transfer(i2c_adap, msgs, ARRAY_SIZE(msgs)); - if (r <= 0) - return r; + if (r
[PATCH 32/40] drm/amdgpu: Return result fix in RAS
The low level EEPROM write method, doesn't return 1, but the number of bytes written. Thus do not compare to 1, instead, compare to greater than 0 for success. Other cleanup: if the lower layers returned -errno, then return that, as opposed to overwriting the error code with one-fits-all -EINVAL. For instance, some return -EAGAIN. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Reviewed-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c| 3 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 22 +++ .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 2 +- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c| 3 +-- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c index a5a87affedabf1..a4815af111ed12 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -105,8 +105,7 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, int r; u16 len; - r = 0; - for ( ; buf_size > 0; + for (r = 0; buf_size > 0; buf_size -= len, eeprom_addr += len, eeprom_buf += len) { /* Set the EEPROM address we want to write to/read from. */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 9a878b5a7665fb..18dee704ccf4dd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -355,8 +355,9 @@ static int amdgpu_ras_debugfs_ctrl_parse_data(struct file *f, * to see which blocks support RAS on a particular asic. * */ -static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f, const char __user *buf, - size_t size, loff_t *pos) +static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f, +const char __user *buf, +size_t size, loff_t *pos) { struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private; struct ras_debug_if data; @@ -370,7 +371,7 @@ static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f, const char __user * ret = amdgpu_ras_debugfs_ctrl_parse_data(f, buf, size, pos, ); if (ret) - return -EINVAL; + return ret; if (data.op == 3) { ret = amdgpu_reserve_page_direct(adev, data.inject.address); @@ -439,21 +440,24 @@ static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f, const char __user * * will reset EEPROM table to 0 entries. * */ -static ssize_t amdgpu_ras_debugfs_eeprom_write(struct file *f, const char __user *buf, - size_t size, loff_t *pos) +static ssize_t amdgpu_ras_debugfs_eeprom_write(struct file *f, + const char __user *buf, + size_t size, loff_t *pos) { struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private; int ret; ret = amdgpu_ras_eeprom_reset_table( - &(amdgpu_ras_get_context(adev)->eeprom_control)); + &(amdgpu_ras_get_context(adev)->eeprom_control)); - if (ret == 1) { + if (ret > 0) { + /* Something was written to EEPROM. +*/ amdgpu_ras_get_context(adev)->flags = RAS_DEFAULT_FLAGS; return size; } else { - return -EIO; + return ret; } } @@ -1991,7 +1995,7 @@ int amdgpu_ras_recovery_init(struct amdgpu_device *adev) kfree(*data); con->eh_data = NULL; out: - dev_warn(adev->dev, "Failed to initialize ras recovery!\n"); + dev_warn(adev->dev, "Failed to initialize ras recovery! (%d)\n", ret); /* * Except error threshold exceeding case, other failure cases in this diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index 17cea35275e46c..dc48c556398039 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -335,7 +335,7 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control, ret = amdgpu_ras_eeprom_reset_table(control); } - return ret == 1 ? 0 : -EIO; + return ret > 0 ? 0 : -EIO; } static void __encode_table_record_to_buff(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 65035256756679..7f48ee020bc03e 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -222,7 +222,7 @@ static uint32_t
[PATCH 26/40] drm/amdgpu: Rename misspelled function
Instead of fixing the spelling in amdgpu_ras_eeprom_process_recods(), rename it to, amdgpu_ras_eeprom_xfer(), to look similar to other I2C and protocol transfer (read/write) functions. Also to keep the column span to within reason by using a shorter name. Change the "num" function parameter from "int" to "const u32" since it is the number of items (records) to xfer, i.e. their count, which cannot be a negative number. Also swap the order of parameters, keeping the pointer to records and their number next to each other, while the direction now becomes the last parameter. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c| 11 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h | 7 +++ 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index beb174f79fa0bd..edd4d83e1e392f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1817,10 +1817,10 @@ int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev) save_count = data->count - control->num_recs; /* only new entries are saved */ if (save_count > 0) { - if (amdgpu_ras_eeprom_process_recods(control, - >bps[control->num_recs], - true, - save_count)) { + if (amdgpu_ras_eeprom_xfer(control, + >bps[control->num_recs], + save_count, + true)) { dev_err(adev->dev, "Failed to save EEPROM table data!"); return -EIO; } @@ -1850,8 +1850,7 @@ static int amdgpu_ras_load_bad_pages(struct amdgpu_device *adev) if (!bps) return -ENOMEM; - if (amdgpu_ras_eeprom_process_recods(control, bps, false, - control->num_recs)) { + if (amdgpu_ras_eeprom_xfer(control, bps, control->num_recs, false)) { dev_err(adev->dev, "Failed to load EEPROM table records!"); ret = -EIO; goto out; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index d3678706bb736d..9e3fbc44b4bc4a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -432,9 +432,9 @@ bool amdgpu_ras_eeprom_check_err_threshold(struct amdgpu_device *adev) return false; } -int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, -struct eeprom_table_record *records, -bool write, int num) +int amdgpu_ras_eeprom_xfer(struct amdgpu_ras_eeprom_control *control, + struct eeprom_table_record *records, + const u32 num, bool write) { int i, ret = 0; unsigned char *buffs, *buff; @@ -574,13 +574,13 @@ void amdgpu_ras_eeprom_test(struct amdgpu_ras_eeprom_control *control) recs[i].retired_page = i; } - if (!amdgpu_ras_eeprom_process_recods(control, recs, true, 1)) { + if (!amdgpu_ras_eeprom_xfer(control, recs, 1, true)) { memset(recs, 0, sizeof(*recs) * 1); control->next_addr = RAS_RECORD_START; - if (!amdgpu_ras_eeprom_process_recods(control, recs, false, 1)) { + if (!amdgpu_ras_eeprom_xfer(control, recs, 1, false)) { for (i = 0; i < 1; i++) DRM_INFO("rec.address :0x%llx, rec.retired_page :%llu", recs[i].address, recs[i].retired_page); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h index 4c4c3d840a35c5..6a1bd527bce57a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h @@ -82,10 +82,9 @@ int amdgpu_ras_eeprom_reset_table(struct amdgpu_ras_eeprom_control *control); bool amdgpu_ras_eeprom_check_err_threshold(struct amdgpu_device *adev); -int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, - struct eeprom_table_record *records, - bool write, - int num); +int amdgpu_ras_eeprom_xfer(struct amdgpu_ras_eeprom_control *control, + struct eeprom_table_record *records, +
[PATCH 14/40] drm/amdgpu: Drop i > 0 restriction for issuing RESTART
From: Andrey Grodzovsky Drop i > 0 restriction for issuing RESTART. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Andrey Grodzovsky Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov --- drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 2 +- 3 files changed, 3 insertions(+), 3 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 843eb2357afaaf..a6d6ea1ef9e31b 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -1984,7 +1984,7 @@ static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap, if ((msg[i].flags & I2C_M_STOP) || (!remaining_bytes)) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; - if ((i > 0) && (j == 0) && !(msg[i].flags & I2C_M_NOSTART)) + if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } } 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 f8219e9ef5c62d..dad322f46db3c9 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2783,7 +2783,7 @@ static int navi10_i2c_xfer(struct i2c_adapter *i2c_adap, if ((msg[i].flags & I2C_M_STOP) || (!remaining_bytes)) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; - if ((i > 0) && (j == 0) && !(msg[i].flags & I2C_M_NOSTART)) + if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } } 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 9e49505a6ac109..93acf3f869227a 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 @@ -3490,7 +3490,7 @@ static int sienna_cichlid_i2c_xfer(struct i2c_adapter *i2c_adap, if ((msg[i].flags & I2C_M_STOP) || (!remaining_bytes)) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; - if ((i > 0) && (j == 0) && !(msg[i].flags & I2C_M_NOSTART)) + if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } } -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 16/40] drm/amd/pm: SMU I2C: Return number of messages processed
From: Andrey Grodzovsky Fix from number of processed bytes to number of processed I2C messages. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Andrey Grodzovsky Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov Reviewed-by: Alexander Deucher --- .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 43 +++ .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 43 +++ .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 43 +++ 3 files changed, 75 insertions(+), 54 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 fde03bb6ffe7c8..c916ccc48bf67f 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -1943,9 +1943,8 @@ static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap, struct smu_table_context *smu_table = >smu.smu_table; struct smu_table *table = _table->driver_table; SwI2cRequest_t *req, *res = (SwI2cRequest_t *)table->cpu_addr; - u16 bytes_to_transfer, remaining_bytes, msg_bytes; - u16 available_bytes = MAX_SW_I2C_COMMANDS; - int i, j, r, c; + 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 */ @@ -1953,8 +1952,15 @@ static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap, for (i = 0; i < num; i++) { if (slave != msgs[i].addr) return -EINVAL; - bytes_to_transfer += min(msgs[i].len, available_bytes); - available_bytes -= bytes_to_transfer; + + 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; + } } req = kzalloc(sizeof(*req), GFP_KERNEL); @@ -1964,24 +1970,28 @@ 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 = bytes_to_transfer; + req->NumCmds = MAX_SW_I2C_COMMANDS - available_bytes;; - remaining_bytes = bytes_to_transfer; c = 0; - for (i = 0; i < num; i++) { + for (i = 0; i < num_done; i++) { struct i2c_msg *msg = [i]; - msg_bytes = min(msg->len, remaining_bytes); - for (j = 0; j < msg_bytes; j++) { + for (j = 0; j < msg->len; j++) { SwI2cCmd_t *cmd = >SwI2cCmds[c++]; - remaining_bytes--; if (!(msg[i].flags & I2C_M_RD)) { /* write */ cmd->CmdConfig |= I2C_CMD_WRITE; cmd->RegisterAddr = msg->buf[j]; } - if (!remaining_bytes) + + /* +* 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))) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART)) @@ -1994,21 +2004,18 @@ static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap, if (r) goto fail; - remaining_bytes = bytes_to_transfer; c = 0; - for (i = 0; i < num; i++) { + for (i = 0; i < num_done; i++) { struct i2c_msg *msg = [i]; - msg_bytes = min(msg->len, remaining_bytes); - for (j = 0; j < msg_bytes; j++) { + for (j = 0; j < msg->len; j++) { SwI2cCmd_t *cmd = >SwI2cCmds[c++]; - remaining_bytes--; if (msg[i].flags & I2C_M_RD) msg->buf[j] = cmd->Data; } } - r = bytes_to_transfer; + r = num_done; fail: kfree(req); 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 666b090e663894..d734f51b1cfaa4 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2742,9 +2742,8 @@ static int navi10_i2c_xfer(struct i2c_adapter *i2c_adap, struct
[PATCH 35/40] drm/amdgpu: Use explicit cardinality for clarity
RAS_MAX_RECORD_NUM may mean the maximum record number, as in the maximum house number on your street, or it may mean the maximum number of records, as in the count of records, which is also a number. To make this distinction whether the number is ordinal (index) or cardinal (count), rename this macro to RAS_MAX_RECORD_COUNT. This makes it easy to understand what it refers to, especially when we compute quantities such as, how many records do we have left in the table, especially when there are so many other numbers, quantities and numerical macros around. Also rename the long, amdgpu_ras_eeprom_get_record_max_length() to the more succinct and clear, amdgpu_ras_eeprom_max_record_count(). When computing the threshold, which also deals with counts, i.e. "how many", use cardinal "max_eeprom_records_count", than the quantitative "max_eeprom_records_len". Simplify the logic here and there, as well. Cc: Guchun Chen Cc: John Clements Cc: Hawking Zhang Cc: Alexander Deucher Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 50 --- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 8 +-- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h| 2 +- 4 files changed, 30 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 3de1accb060e37..0203f654576bcc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -853,11 +853,10 @@ MODULE_PARM_DESC(reset_method, "GPU reset method (-1 = auto (default), 0 = legac module_param_named(reset_method, amdgpu_reset_method, int, 0444); /** - * DOC: bad_page_threshold (int) - * Bad page threshold is to specify the threshold value of faulty pages - * detected by RAS ECC, that may result in GPU entering bad status if total - * faulty pages by ECC exceed threshold value and leave it for user's further - * check. + * DOC: bad_page_threshold (int) Bad page threshold is specifies the + * threshold value of faulty pages detected by RAS ECC, which may + * result in the GPU entering bad status when the number of total + * faulty pages by ECC exceeds the threshold value. */ MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default value), 0 = disable bad page retirement)"); module_param_named(bad_page_threshold, amdgpu_bad_page_threshold, int, 0444); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 18dee704ccf4dd..ecdf2d80eee8f0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -71,8 +71,8 @@ const char *ras_block_string[] = { /* inject address is 52 bits */ #defineRAS_UMC_INJECT_ADDR_LIMIT (0x1ULL << 52) -/* typical ECC bad page rate(1 bad page per 100MB VRAM) */ -#define RAS_BAD_PAGE_RATE (100 * 1024 * 1024ULL) +/* typical ECC bad page rate is 1 bad page per 100MB VRAM */ +#define RAS_BAD_PAGE_COVER (100 * 1024 * 1024ULL) enum amdgpu_ras_retire_page_reservation { AMDGPU_RAS_RETIRE_PAGE_RESERVED, @@ -1841,27 +1841,24 @@ int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev) static int amdgpu_ras_load_bad_pages(struct amdgpu_device *adev) { struct amdgpu_ras_eeprom_control *control = - >psp.ras.ras->eeprom_control; - struct eeprom_table_record *bps = NULL; - int ret = 0; + >psp.ras.ras->eeprom_control; + struct eeprom_table_record *bps; + int ret; /* no bad page record, skip eeprom access */ - if (!control->num_recs || (amdgpu_bad_page_threshold == 0)) - return ret; + if (control->num_recs == 0 || amdgpu_bad_page_threshold == 0) + return 0; bps = kcalloc(control->num_recs, sizeof(*bps), GFP_KERNEL); if (!bps) return -ENOMEM; - if (amdgpu_ras_eeprom_read(control, bps, control->num_recs)) { + ret = amdgpu_ras_eeprom_read(control, bps, control->num_recs); + if (ret) dev_err(adev->dev, "Failed to load EEPROM table records!"); - ret = -EIO; - goto out; - } - - ret = amdgpu_ras_add_bad_pages(adev, bps, control->num_recs); + else + ret = amdgpu_ras_add_bad_pages(adev, bps, control->num_recs); -out: kfree(bps); return ret; } @@ -1901,11 +1898,9 @@ static bool amdgpu_ras_check_bad_page(struct amdgpu_device *adev, } static void amdgpu_ras_validate_threshold(struct amdgpu_device *adev, - uint32_t max_length) + uint32_t max_count) { struct amdgpu_ras *con = amdgpu_ras_get_context(adev); - int tmp_threshold = amdgpu_bad_page_threshold; - u64 val; /* *
[PATCH 12/40] drm/amdgpu: Remember to wait 10ms for write buffer flush v2
From: Andrey Grodzovsky EEPROM spec requests this. v2: Only to be done for write data transactions. Signed-off-by: Andrey Grodzovsky Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c index 10551660343278..fe0e9b0c4d5a38 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -55,6 +55,21 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, r = i2c_transfer(i2c_adap, msgs, ARRAY_SIZE(msgs)); if (r <= 0) return r; + + /* Only for write data */ + if (!msgs[1].flags) + /* +* 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. +* +* TODO Improve to wait for first ACK for slave address after +* internal write cycle done. +*/ + msleep(10); + + bytes_transferred = r - EEPROM_OFFSET_LENGTH; eeprom_addr += bytes_transferred; msgs[0].buf[0] = ((eeprom_addr >> 8) & 0xff); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 21/40] drm/amdgpu: I2C EEPROM full memory addressing
* "eeprom_addr" is now 32-bit wide. * Remove "slave_addr" from the I2C EEPROM driver interface. The I2C EEPROM Device Type Identifier is fixed at 1010b, and the rest of the bits of the Device Address Byte/Device Select Code, are memory address bits, where the first three of those bits are the hardware selection bits. All this is now a 19-bit address and passed as "eeprom_addr". This abstracts the I2C bus for EEPROM devices for this I2C EEPROM driver. Now clients only pass the 19-bit EEPROM memory address, to the I2C EEPROM driver, as the 32-bit "eeprom_addr", from which they want to read from or write to. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 88 +- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h | 4 +- 2 files changed, 72 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c index 94aeda1c7f8ca0..a5a87affedabf1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -24,7 +24,7 @@ #include "amdgpu_eeprom.h" #include "amdgpu.h" -/* AT24CM02 has a 256-byte write page size. +/* AT24CM02 and M24M02-R have a 256-byte write page size. */ #define EEPROM_PAGE_BITS 8 #define EEPROM_PAGE_SIZE (1U << EEPROM_PAGE_BITS) @@ -32,20 +32,72 @@ #define EEPROM_OFFSET_SIZE 2 -static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, - u16 slave_addr, u16 eeprom_addr, +/* EEPROM memory addresses are 19-bits long, which can + * be partitioned into 3, 8, 8 bits, for a total of 19. + * The upper 3 bits are sent as part of the 7-bit + * "Device Type Identifier"--an I2C concept, which for EEPROM devices + * is hard-coded as 1010b, indicating that it is an EEPROM + * device--this is the wire format, followed by the upper + * 3 bits of the 19-bit address, followed by the direction, + * followed by two bytes holding the rest of the 16-bits of + * the EEPROM memory address. The format on the wire for EEPROM + * devices is: 1010XYZD, A15:A8, A7:A0, + * Where D is the direction and sequenced out by the hardware. + * Bits XYZ are memory address bits 18, 17 and 16. + * These bits are compared to how pins 1-3 of the part are connected, + * depending on the size of the part, more on that later. + * + * Note that of this wire format, a client is in control + * of, and needs to specify only XYZ, A15:A8, A7:0, bits, + * which is exactly the EEPROM memory address, or offset, + * in order to address up to 8 EEPROM devices on the I2C bus. + * + * For instance, a 2-Mbit I2C EEPROM part, addresses all its bytes, + * using an 18-bit address, bit 17 to 0 and thus would use all but one bit of + * the 19 bits previously mentioned. The designer would then not connect + * pins 1 and 2, and pin 3 usually named "A_2" or "E2", would be connected to + * either Vcc or GND. This would allow for up to two 2-Mbit parts on + * the same bus, where one would be addressable with bit 18 as 1, and + * the other with bit 18 of the address as 0. + * + * For a 2-Mbit part, bit 18 is usually known as the "Chip Enable" or + * "Hardware Address Bit". This bit is compared to the load on pin 3 + * of the device, described above, and if there is a match, then this + * device responds to the command. This way, you can connect two + * 2-Mbit EEPROM devices on the same bus, but see one contiguous + * memory from 0 to 7h, where address 0 to 3 is in the device + * whose pin 3 is connected to GND, and address 4 to 7h is in + * the 2nd device, whose pin 3 is connected to Vcc. + * + * This addressing you encode in the 32-bit "eeprom_addr" below, + * namely the 19-bits "XYZ,A15:A0", as a single 19-bit address. For + * instance, eeprom_addr = 0x6DA01, is 110_1101_1010__0001, where + * XYZ=110b, and A15:A0=DA01h. The XYZ bits become part of the device + * address, and the rest of the address bits are sent as the memory + * address bytes. + * + * That is, for an I2C EEPROM driver everything is controlled by + * the "eeprom_addr". + * + * P.S. If you need to write, lock and read the Identification Page, + * (M24M02-DR device only, which we do not use), change the "7" to + * "0xF" in the macro below, and let the client set bit 20 to 1 in + * "eeprom_addr", and set A10 to 0 to write into it, and A10 and A1 to + * 1 to lock it permanently. + */ +#define MAKE_I2C_ADDR(_aa) ((0xA << 3) | (((_aa) >> 16) & 7)) + +static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, u8 *eeprom_buf, u16 buf_size, bool read) { u8 eeprom_offset_buf[EEPROM_OFFSET_SIZE]; struct i2c_msg msgs[] = { { - .addr = slave_addr, .flags = 0, .len
[PATCH 24/40] drm/amdgpu: I2C class is HWMON
Set the auto-discoverable class of I2C bus to HWMON. Remove SPD. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Reviewed-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) 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 b8d6d308fb06a0..e403ba556e5590 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -667,7 +667,7 @@ int smu_v11_0_i2c_control_init(struct i2c_adapter *control) mutex_init(>pm.smu_i2c_mutex); control->owner = THIS_MODULE; - control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; + control->class = I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _v11_0_i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); 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 33210119a28ec1..9fccdd2d3e73ec 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -2046,7 +2046,7 @@ static int arcturus_i2c_control_init(struct smu_context *smu, struct i2c_adapter int res; control->owner = THIS_MODULE; - control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; + control->class = I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _i2c_algo; control->quirks = _i2c_control_quirks; 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 22b31055461eb3..e3cbd334a956d1 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2844,7 +2844,7 @@ static int navi10_i2c_control_init(struct smu_context *smu, struct i2c_adapter * int res; control->owner = THIS_MODULE; - control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; + control->class = I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); 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 9a14103cf9729f..2444e13c4901b3 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 @@ -3551,7 +3551,7 @@ static int sienna_cichlid_i2c_control_init(struct smu_context *smu, struct i2c_a int res; control->owner = THIS_MODULE; - control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; + control->class = I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _cichlid_i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 15/40] drm/amdgpu: Send STOP for the last byte of msg only
From: Andrey Grodzovsky Let's just ignore the I2C_M_STOP hint from upper layer for SMU I2C code as there is no clean mapping between single per I2C message STOP flag at the kernel I2C layer and the SMU, per each byte STOP flag. We will just by default set it at the end of the SMU I2C message. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Andrey Grodzovsky Suggested-by: Lazar Lijo Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 4 ++-- drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 4 ++-- drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 4 ++-- 3 files changed, 6 insertions(+), 6 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 a6d6ea1ef9e31b..fde03bb6ffe7c8 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -1981,9 +1981,9 @@ static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap, cmd->CmdConfig |= I2C_CMD_WRITE; cmd->RegisterAddr = msg->buf[j]; } - if ((msg[i].flags & I2C_M_STOP) || - (!remaining_bytes)) + if (!remaining_bytes) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; + if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } 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 dad322f46db3c9..666b090e663894 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2780,9 +2780,9 @@ static int navi10_i2c_xfer(struct i2c_adapter *i2c_adap, cmd->CmdConfig |= I2C_CMD_WRITE; cmd->RegisterAddr = msg->buf[j]; } - if ((msg[i].flags & I2C_M_STOP) || - (!remaining_bytes)) + if (!remaining_bytes) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; + if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } 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 93acf3f869227a..7c266420e31cc7 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 @@ -3487,9 +3487,9 @@ static int sienna_cichlid_i2c_xfer(struct i2c_adapter *i2c_adap, cmd->CmdConfig |= CMDCONFIG_READWRITE_MASK; cmd->ReadWriteData = msg->buf[j]; } - if ((msg[i].flags & I2C_M_STOP) || - (!remaining_bytes)) + if (!remaining_bytes) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; + if ((j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 28/40] drm/amdgpu: EEPROM: add explicit read and write
Add explicit amdgpu_eeprom_read() and amdgpu_eeprom_write() for clarity. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h | 16 drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 5 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 10 +- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h index 417472be2712e6..966b434f0de2b7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h @@ -29,4 +29,20 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr, u8 *eeprom_buf, u16 bytes, bool read); +static inline int amdgpu_eeprom_read(struct i2c_adapter *i2c_adap, +u32 eeprom_addr, u8 *eeprom_buf, +u16 bytes) +{ + return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes, + true); +} + +static inline int amdgpu_eeprom_write(struct i2c_adapter *i2c_adap, + u32 eeprom_addr, u8 *eeprom_buf, + u16 bytes) +{ + return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes, + false); +} + #endif diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c index 69b9559f840ac3..7709caeb233d67 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c @@ -66,7 +66,7 @@ static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr, { int ret, size; - ret = amdgpu_eeprom_xfer(>pm.smu_i2c, addrptr, buff, 1, true); + ret = amdgpu_eeprom_read(>pm.smu_i2c, addrptr, buff, 1); if (ret < 1) { DRM_WARN("FRU: Failed to get size field"); return ret; @@ -77,8 +77,7 @@ static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr, */ size = buff[0] - I2C_PRODUCT_INFO_OFFSET; - ret = amdgpu_eeprom_xfer(>pm.smu_i2c, addrptr + 1, buff, size, -true); + ret = amdgpu_eeprom_read(>pm.smu_i2c, addrptr + 1, buff, size); if (ret < 1) { DRM_WARN("FRU: Failed to get data field"); return ret; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index 550a31953d2da1..17cea35275e46c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -151,9 +151,9 @@ static int __update_table_header(struct amdgpu_ras_eeprom_control *control, /* i2c may be unstable in gpu reset */ down_read(>reset_sem); - ret = amdgpu_eeprom_xfer(>pm.smu_i2c, -control->i2c_address + RAS_HDR_START, -buff, RAS_TABLE_HEADER_SIZE, false); + ret = amdgpu_eeprom_write(>pm.smu_i2c, + control->i2c_address + RAS_HDR_START, + buff, RAS_TABLE_HEADER_SIZE); up_read(>reset_sem); if (ret < 1) @@ -298,9 +298,9 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control, mutex_init(>tbl_mutex); /* Read/Create table header from EEPROM address 0 */ - ret = amdgpu_eeprom_xfer(>pm.smu_i2c, + ret = amdgpu_eeprom_read(>pm.smu_i2c, control->i2c_address + RAS_HDR_START, -buff, RAS_TABLE_HEADER_SIZE, true); +buff, RAS_TABLE_HEADER_SIZE); if (ret < 1) { DRM_ERROR("Failed to read EEPROM table header, ret:%d", ret); return ret; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 17/40] drm/amdgpu/pm: ADD I2C quirk adapter table
From: Andrey Grodzovsky To be used by kernel clients of the adapter. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Andrey Grodzovsky Suggested-by: Lazar Lijo Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov Reviewed-by: Alexander Deucher --- drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 7 +++ drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 6 ++ drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 6 ++ 3 files changed, 19 insertions(+) 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 c916ccc48bf67f..33210119a28ec1 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -2034,6 +2034,12 @@ static const struct i2c_algorithm arcturus_i2c_algo = { .functionality = arcturus_i2c_func, }; + +static const struct i2c_adapter_quirks arcturus_i2c_control_quirks = { + .max_read_len = MAX_SW_I2C_COMMANDS, + .max_write_len = MAX_SW_I2C_COMMANDS, +}; + static int arcturus_i2c_control_init(struct smu_context *smu, struct i2c_adapter *control) { struct amdgpu_device *adev = to_amdgpu_device(control); @@ -2043,6 +2049,7 @@ static int arcturus_i2c_control_init(struct smu_context *smu, struct i2c_adapter control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _i2c_algo; + control->quirks = _i2c_control_quirks; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); res = i2c_add_adapter(control); 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 d734f51b1cfaa4..22b31055461eb3 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2833,6 +2833,11 @@ static const struct i2c_algorithm navi10_i2c_algo = { .functionality = navi10_i2c_func, }; +static const struct i2c_adapter_quirks navi10_i2c_control_quirks = { + .max_read_len = MAX_SW_I2C_COMMANDS, + .max_write_len = MAX_SW_I2C_COMMANDS, +}; + static int navi10_i2c_control_init(struct smu_context *smu, struct i2c_adapter *control) { struct amdgpu_device *adev = to_amdgpu_device(control); @@ -2843,6 +2848,7 @@ static int navi10_i2c_control_init(struct smu_context *smu, struct i2c_adapter * control->dev.parent = >pdev->dev; control->algo = _i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); + control->quirks = _i2c_control_quirks; res = i2c_add_adapter(control); if (res) 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 8f8e5c7df44a12..9a14103cf9729f 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 @@ -3540,6 +3540,11 @@ static const struct i2c_algorithm sienna_cichlid_i2c_algo = { .functionality = sienna_cichlid_i2c_func, }; +static const struct i2c_adapter_quirks sienna_cichlid_i2c_control_quirks = { + .max_read_len = MAX_SW_I2C_COMMANDS, + .max_write_len = MAX_SW_I2C_COMMANDS, +}; + static int sienna_cichlid_i2c_control_init(struct smu_context *smu, struct i2c_adapter *control) { struct amdgpu_device *adev = to_amdgpu_device(control); @@ -3550,6 +3555,7 @@ static int sienna_cichlid_i2c_control_init(struct smu_context *smu, struct i2c_a control->dev.parent = >pdev->dev; control->algo = _cichlid_i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); + control->quirks = _cichlid_i2c_control_quirks; res = i2c_add_adapter(control); if (res) -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 30/40] drm/amd/pm: Simplify managed I2C transfer functions
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 Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- .../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 3286c704bd08df..dc884b719717ad 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -1937,31 +1937,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 = >smu.smu_table; struct smu_table *table = _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) @@ -1969,33 +1952,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 = [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 = >SwI2cCmds[c++]; + for (c = i = 0; i < num_msgs; i++) { + for (j = 0; j < msg[i].len; j++, c++) { + SwI2cCmd_t *cmd = >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; - -
[PATCH 20/40] drm/amdgpu: EEPROM respects I2C quirks
Consult the i2c_adapter.quirks table for the maximum read/write data length per bus transaction. Do not exceed this transaction limit. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 80 +- 1 file changed, 64 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c index 7fdb5bd2fc8bc8..94aeda1c7f8ca0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -32,20 +32,9 @@ #define EEPROM_OFFSET_SIZE 2 -/** - * amdgpu_eeprom_xfer -- Read/write from/to an I2C EEPROM device - * @i2c_adap: pointer to the I2C adapter to use - * @slave_addr: I2C address of the slave device - * @eeprom_addr: EEPROM address from which to read/write - * @eeprom_buf: pointer to data buffer to read into/write from - * @buf_size: the size of @eeprom_buf - * @read: True if reading from the EEPROM, false if writing - * - * Returns the number of bytes read/written; -errno on error. - */ -int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, - u16 slave_addr, u16 eeprom_addr, - u8 *eeprom_buf, u16 buf_size, bool read) +static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, + u16 slave_addr, u16 eeprom_addr, + u8 *eeprom_buf, u16 buf_size, bool read) { u8 eeprom_offset_buf[EEPROM_OFFSET_SIZE]; struct i2c_msg msgs[] = { @@ -65,8 +54,8 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u16 len; r = 0; - for (len = 0; buf_size > 0; -buf_size -= len, eeprom_addr += len, eeprom_buf += len) { + for ( ; buf_size > 0; + buf_size -= len, eeprom_addr += len, eeprom_buf += len) { /* Set the EEPROM address we want to write to/read from. */ msgs[0].buf[0] = (eeprom_addr >> 8) & 0xff; @@ -120,3 +109,62 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, return r < 0 ? r : eeprom_buf - p; } + +/** + * amdgpu_eeprom_xfer -- Read/write from/to an I2C EEPROM device + * @i2c_adap: pointer to the I2C adapter to use + * @slave_addr: I2C address of the slave device + * @eeprom_addr: EEPROM address from which to read/write + * @eeprom_buf: pointer to data buffer to read into/write from + * @buf_size: the size of @eeprom_buf + * @read: True if reading from the EEPROM, false if writing + * + * Returns the number of bytes read/written; -errno on error. + */ +int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, + u16 slave_addr, u16 eeprom_addr, + u8 *eeprom_buf, u16 buf_size, bool read) +{ + const struct i2c_adapter_quirks *quirks = i2c_adap->quirks; + u16 limit; + + if (!quirks) + limit = 0; + else if (read) + limit = quirks->max_read_len; + else + limit = quirks->max_write_len; + + if (limit == 0) { + return __amdgpu_eeprom_xfer(i2c_adap, slave_addr, eeprom_addr, + eeprom_buf, buf_size, read); + } else if (limit <= EEPROM_OFFSET_SIZE) { + dev_err_ratelimited(_adap->dev, + "maddr:0x%04X size:0x%02X:quirk max_%s_len must be > %d", + eeprom_addr, buf_size, + read ? "read" : "write", EEPROM_OFFSET_SIZE); + return -EINVAL; + } else { + u16 ps; /* Partial size */ + int res = 0, r; + + /* The "limit" includes all data bytes sent/received, +* which would include the EEPROM_OFFSET_SIZE bytes. +* Account for them here. +*/ + limit -= EEPROM_OFFSET_SIZE; + for ( ; buf_size > 0; + buf_size -= ps, eeprom_addr += ps, eeprom_buf += ps) { + ps = min(limit, buf_size); + + r = __amdgpu_eeprom_xfer(i2c_adap, +slave_addr, eeprom_addr, +eeprom_buf, ps, read); + if (r < 0) + return r; + res += r; + } + + return res; + } +} -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 18/40] drm/amdgpu: Fix Vega20 I2C to be agnostic (v2)
Teach Vega20 I2C to be agnostic. Allow addressing different devices while the master holds the bus. Set STOP as per the controller's specification. v2: Qualify generating ReSTART before the 1st byte of the message, when set by the caller, as those functions are separated, as caught by Andrey G. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 4 +- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 105 + 2 files changed, 69 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c index fe0e9b0c4d5a38..d02ea083a6c69b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -41,10 +41,10 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, }, { .addr = slave_addr, - .flags = read ? I2C_M_RD: 0, + .flags = read ? I2C_M_RD : 0, .len = bytes, .buf = eeprom_buf, - } + }, }; int r; 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 5a90d9351b22eb..b8d6d308fb06a0 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -41,9 +41,7 @@ #define I2C_SW_TIMEOUT8 #define I2C_ABORT 0x10 -/* I2C transaction flags */ -#define I2C_NO_STOP1 -#define I2C_RESTART2 +#define I2C_X_RESTART BIT(31) #define to_amdgpu_device(x) (container_of(x, struct amdgpu_device, pm.smu_i2c)) @@ -205,9 +203,6 @@ static uint32_t smu_v11_0_i2c_poll_rx_status(struct i2c_adapter *control) return ret; } - - - /** * smu_v11_0_i2c_transmit - Send a block of data over the I2C bus to a slave device. * @@ -252,21 +247,22 @@ static uint32_t smu_v11_0_i2c_transmit(struct i2c_adapter *control, reg = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_STATUS); if (REG_GET_FIELD(reg, CKSVII2C_IC_STATUS, TFNF)) { do { - reg = 0; - /* -* Prepare transaction, no need to set RESTART. I2C engine will send -* START as soon as it sees data in TXFIFO -*/ - if (bytes_sent == 0) - reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, RESTART, - (i2c_flag & I2C_RESTART) ? 1 : 0); reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, DAT, data[bytes_sent]); - /* determine if we need to send STOP bit or not */ - if (numbytes == 1) - /* Final transaction, so send stop unless I2C_NO_STOP */ - reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, STOP, - (i2c_flag & I2C_NO_STOP) ? 0 : 1); + /* Final message, final byte, must +* generate a STOP, to release the +* bus, i.e. don't hold SCL low. +*/ + if (numbytes == 1 && i2c_flag & I2C_M_STOP) + reg = REG_SET_FIELD(reg, + CKSVII2C_IC_DATA_CMD, + STOP, 1); + + if (bytes_sent == 0 && i2c_flag & I2C_X_RESTART) + reg = REG_SET_FIELD(reg, + CKSVII2C_IC_DATA_CMD, + RESTART, 1); + /* Write */ reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, CMD, 0); WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_DATA_CMD, reg); @@ -341,23 +337,21 @@ static uint32_t smu_v11_0_i2c_receive(struct i2c_adapter *control, smu_v11_0_i2c_clear_status(control); - /* Prepare transaction */ - - /* Each time we disable I2C, so this is not a restart */ - if (bytes_received == 0) - reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, RESTART, - (i2c_flag & I2C_RESTART) ? 1 : 0); - reg = REG_SET_FIELD(reg, CKSVII2C_IC_DATA_CMD, DAT, 0);
[PATCH 08/40] drm/amdgpu: i2c subsystem uses 7 bit addresses
From: Alex Deucher Convert from 8 bit to 7 bit. Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 10 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c index 224da573ba1b59..2b854bc6ae34bb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c @@ -29,7 +29,7 @@ #include "amdgpu_fru_eeprom.h" #include "amdgpu_eeprom.h" -#define I2C_PRODUCT_INFO_ADDR 0xAC +#define I2C_PRODUCT_INFO_ADDR 0x56 #define I2C_PRODUCT_INFO_OFFSET0xC0 static bool is_fru_eeprom_supported(struct amdgpu_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 e22a0b45f70108..2b981e96ce5b9e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -28,11 +28,11 @@ #include "atom.h" #include "amdgpu_eeprom.h" -#define EEPROM_I2C_TARGET_ADDR_VEGA20 0xA0 -#define EEPROM_I2C_TARGET_ADDR_ARCTURUS0xA8 -#define EEPROM_I2C_TARGET_ADDR_ARCTURUS_D342 0xA0 -#define EEPROM_I2C_TARGET_ADDR_SIENNA_CICHLID 0xA0 -#define EEPROM_I2C_TARGET_ADDR_ALDEBARAN0xA0 +#define EEPROM_I2C_TARGET_ADDR_VEGA20 0x50 +#define EEPROM_I2C_TARGET_ADDR_ARCTURUS0x54 +#define EEPROM_I2C_TARGET_ADDR_ARCTURUS_D342 0x50 +#define EEPROM_I2C_TARGET_ADDR_SIENNA_CICHLID 0x50 +#define EEPROM_I2C_TARGET_ADDR_ALDEBARAN0x50 /* * The 2 macros bellow represent the actual size in bytes that -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 13/40] dmr/amdgpu: Add RESTART handling also to smu_v11_0_i2c (VG20)
From: Andrey Grodzovsky Also generilize the code to accept and translate to HW bits any I2C relvent flags both for read and write. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Andrey Grodzovsky Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) 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 3193d566f4f87e..5a90d9351b22eb 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -530,13 +530,11 @@ static bool smu_v11_0_i2c_bus_unlock(struct i2c_adapter *control) /* I2C GLUE / static uint32_t smu_v11_0_i2c_read_data(struct i2c_adapter *control, - struct i2c_msg *msg) + struct i2c_msg *msg, uint32_t i2c_flag) { - uint32_t ret = 0; + uint32_t ret; - /* Now read data starting with that address */ - ret = smu_v11_0_i2c_receive(control, msg->addr, msg->buf, msg->len, - I2C_RESTART); + ret = smu_v11_0_i2c_receive(control, msg->addr, msg->buf, msg->len, i2c_flag); if (ret != I2C_OK) DRM_ERROR("ReadData() - I2C error occurred :%x", ret); @@ -545,12 +543,11 @@ static uint32_t smu_v11_0_i2c_read_data(struct i2c_adapter *control, } static uint32_t smu_v11_0_i2c_write_data(struct i2c_adapter *control, - struct i2c_msg *msg) + struct i2c_msg *msg, uint32_t i2c_flag) { uint32_t ret; - /* Send I2C_NO_STOP unless requested to stop. */ - ret = smu_v11_0_i2c_transmit(control, msg->addr, msg->buf, msg->len, ((msg->flags & I2C_M_STOP) ? 0 : I2C_NO_STOP)); + ret = smu_v11_0_i2c_transmit(control, msg->addr, msg->buf, msg->len, i2c_flag); if (ret != I2C_OK) DRM_ERROR("WriteI2CData() - I2C error occurred :%x", ret); @@ -601,12 +598,17 @@ static int smu_v11_0_i2c_xfer(struct i2c_adapter *i2c_adap, smu_v11_0_i2c_init(i2c_adap); for (i = 0; i < num; i++) { + uint32_t i2c_flag = ((msgs[i].flags & I2C_M_NOSTART) ? 0 : I2C_RESTART) || + (((msgs[i].flags & I2C_M_STOP) ? 0 : I2C_NO_STOP)); + if (msgs[i].flags & I2C_M_RD) ret = smu_v11_0_i2c_read_data(i2c_adap, - msgs + i); + msgs + i, + i2c_flag); else ret = smu_v11_0_i2c_write_data(i2c_adap, - msgs + i); + msgs + i, + i2c_flag); if (ret != I2C_OK) { num = -EIO; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 03/40] drm/amdgpu/pm: rework i2c xfers on arcturus (v4)
From: Alex Deucher Make it generic so we can support more than just EEPROMs. v2: fix restart handling between transactions. v3: handle 7 to 8 bit addr conversion v4: Fix --> req. (Luben T) Signed-off-by: Alex Deucher Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov --- .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 229 +- 1 file changed, 58 insertions(+), 171 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 094df6f87cfc41..a6447822deb09f 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -1936,197 +1936,84 @@ static int arcturus_dpm_set_vcn_enable(struct smu_context *smu, bool enable) return ret; } -static void arcturus_fill_i2c_req(SwI2cRequest_t *req, bool write, - uint8_t address, uint32_t numbytes, - uint8_t *data) -{ - int i; - - req->I2CcontrollerPort = 0; - req->I2CSpeed = 2; - req->SlaveAddress = address; - req->NumCmds = numbytes; - - for (i = 0; i < numbytes; i++) { - SwI2cCmd_t *cmd = >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_read_data(struct i2c_adapter *control, - uint8_t address, - uint8_t *data, - uint32_t numbytes) +static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap, +struct i2c_msg *msgs, int num) { - uint32_t i, ret = 0; - SwI2cRequest_t req; - struct amdgpu_device *adev = to_amdgpu_device(control); + struct amdgpu_device *adev = to_amdgpu_device(i2c_adap); struct smu_table_context *smu_table = >smu.smu_table; struct smu_table *table = _table->driver_table; - - if (numbytes > MAX_SW_I2C_COMMANDS) { - dev_err(adev->dev, "numbytes requested %d is over max allowed %d\n", - numbytes, MAX_SW_I2C_COMMANDS); - return -EINVAL; + SwI2cRequest_t *req, *res = (SwI2cRequest_t *)table->cpu_addr; + u16 bytes_to_transfer, remaining_bytes, msg_bytes; + u16 available_bytes = MAX_SW_I2C_COMMANDS; + int i, j, r, c; + 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; + bytes_to_transfer += min(msgs[i].len, available_bytes); + available_bytes -= bytes_to_transfer; } - memset(, 0, sizeof(req)); - arcturus_fill_i2c_req(, false, address, numbytes, data); - - mutex_lock(>smu.mutex); - /* Now read data starting with that address */ - ret = smu_cmn_update_table(>smu, SMU_TABLE_I2C_COMMANDS, 0, , - true); - mutex_unlock(>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; - - dev_dbg(adev->dev, "arcturus_i2c_read_data, address = %x, bytes = %d, data :", - (uint16_t)address, numbytes); + req = kzalloc(sizeof(*req), GFP_KERNEL); + if (!req) + return -ENOMEM; - print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_NONE, - 8, 1, data, numbytes, false); - } else - dev_err(adev->dev, "arcturus_i2c_read_data - error occurred :%x", ret); + req->I2CcontrollerPort = 1; + req->I2CSpeed = I2C_SPEED_FAST_400K; + req->SlaveAddress = slave << 1; /* 8 bit addresses */ + req->NumCmds = bytes_to_transfer; - return ret; -} + remaining_bytes = bytes_to_transfer; + c = 0; + for (i = 0; i < num; i++) { + struct i2c_msg *msg = [i]; -static int arcturus_i2c_write_data(struct i2c_adapter *control, - uint8_t address, -
[PATCH 01/40] drm/amdgpu: add a mutex for the smu11 i2c bus (v2)
From: Alex Deucher So we lock software as well as hardware access to the bus. v2: fix mutex handling. Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 19 +-- drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h| 1 + 2 files changed, 10 insertions(+), 10 deletions(-) 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 5c7d769aee3fba..1d8f6d5180e099 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -584,12 +584,11 @@ static void lock_bus(struct i2c_adapter *i2c, unsigned int flags) { struct amdgpu_device *adev = to_amdgpu_device(i2c); - if (!smu_v11_0_i2c_bus_lock(i2c)) { + mutex_lock(>pm.smu_i2c_mutex); + if (!smu_v11_0_i2c_bus_lock(i2c)) DRM_ERROR("Failed to lock the bus from SMU"); - return; - } - - adev->pm.bus_locked = true; + else + adev->pm.bus_locked = true; } static int trylock_bus(struct i2c_adapter *i2c, unsigned int flags) @@ -602,12 +601,11 @@ static void unlock_bus(struct i2c_adapter *i2c, unsigned int flags) { struct amdgpu_device *adev = to_amdgpu_device(i2c); - if (!smu_v11_0_i2c_bus_unlock(i2c)) { + if (!smu_v11_0_i2c_bus_unlock(i2c)) DRM_ERROR("Failed to unlock the bus from SMU"); - return; - } - - adev->pm.bus_locked = false; + else + adev->pm.bus_locked = false; + mutex_unlock(>pm.smu_i2c_mutex); } static const struct i2c_lock_operations smu_v11_0_i2c_i2c_lock_ops = { @@ -665,6 +663,7 @@ int smu_v11_0_i2c_control_init(struct i2c_adapter *control) struct amdgpu_device *adev = to_amdgpu_device(control); int res; + mutex_init(>pm.smu_i2c_mutex); control->owner = THIS_MODULE; control->class = I2C_CLASS_SPD; control->dev.parent = >pdev->dev; diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h index f6e0e7d8a00771..d03e6fa2bf1adf 100644 --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h @@ -450,6 +450,7 @@ struct amdgpu_pm { /* Used for I2C access to various EEPROMs on relevant ASICs */ struct i2c_adapter smu_i2c; + struct mutexsmu_i2c_mutex; struct list_headpm_attr_list; }; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 11/40] drm/amdgpu: only set restart on first cmd of the smu i2c transaction
From: Alex Deucher Not sure how the firmware interprets these. Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 2 +- 3 files changed, 3 insertions(+), 3 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 404afc9979c69b..843eb2357afaaf 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -1984,7 +1984,7 @@ static int arcturus_i2c_xfer(struct i2c_adapter *i2c_adap, if ((msg[i].flags & I2C_M_STOP) || (!remaining_bytes)) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; - if ((i > 0) && !(msg[i].flags & I2C_M_NOSTART)) + if ((i > 0) && (j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } } 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 d3efe897ebbb2f..f8219e9ef5c62d 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2783,7 +2783,7 @@ static int navi10_i2c_xfer(struct i2c_adapter *i2c_adap, if ((msg[i].flags & I2C_M_STOP) || (!remaining_bytes)) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; - if ((i > 0) && !(msg[i].flags & I2C_M_NOSTART)) + if ((i > 0) && (j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } } 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 1d06641ad87890..9e49505a6ac109 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 @@ -3490,7 +3490,7 @@ static int sienna_cichlid_i2c_xfer(struct i2c_adapter *i2c_adap, if ((msg[i].flags & I2C_M_STOP) || (!remaining_bytes)) cmd->CmdConfig |= CMDCONFIG_STOP_MASK; - if ((i > 0) && !(msg[i].flags & I2C_M_NOSTART)) + if ((i > 0) && (j == 0) && !(msg[i].flags & I2C_M_NOSTART)) cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; } } -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 00/40] I2C fixes (revision 1)
I2C fixes from various people. Some RAS touch-ups too. A rebased tree can also be found here: https://gitlab.freedesktop.org/ltuikov/linux/-/commits/i2c-rework-luben Tested on Vega20 and Sienna Cichlid. This first revision includes acks, squashes rev 1 patch 33 by absolving it into earlier commits it fixes, and includes a new patch, patch 40 to deal with driver aborts seen on large writes to an I2C EEPROM device. Aaron Rice (1): drm/amdgpu: rework smu11 i2c for generic operation Alex Deucher (10): drm/amdgpu: add a mutex for the smu11 i2c bus (v2) drm/amdgpu/pm: rework i2c xfers on sienna cichlid (v4) drm/amdgpu/pm: rework i2c xfers on arcturus (v4) drm/amdgpu/pm: add smu i2c implementation for navi1x (v4) drm/amdgpu: add new helper for handling EEPROM i2c transfers drm/amdgpu/ras: switch ras eeprom handling to use generic helper drm/amdgpu/ras: switch fru eeprom handling to use generic helper (v2) drm/amdgpu: i2c subsystem uses 7 bit addresses drm/amdgpu: add I2C_CLASS_HWMON to SMU i2c buses drm/amdgpu: only set restart on first cmd of the smu i2c transaction Andrey Grodzovsky (6): drm/amdgpu: Remember to wait 10ms for write buffer flush v2 dmr/amdgpu: Add RESTART handling also to smu_v11_0_i2c (VG20) drm/amdgpu: Drop i > 0 restriction for issuing RESTART drm/amdgpu: Send STOP for the last byte of msg only drm/amd/pm: SMU I2C: Return number of messages processed drm/amdgpu/pm: ADD I2C quirk adapter table Luben Tuikov (23): drm/amdgpu: Fix Vega20 I2C to be agnostic (v2) drm/amdgpu: Fixes to the AMDGPU EEPROM driver drm/amdgpu: EEPROM respects I2C quirks drm/amdgpu: I2C EEPROM full memory addressing drm/amdgpu: RAS and FRU now use 19-bit I2C address drm/amdgpu: Fix wrap-around bugs in RAS drm/amdgpu: I2C class is HWMON drm/amdgpu: RAS: EEPROM --> RAS drm/amdgpu: Rename misspelled function drm/amdgpu: RAS xfer to read/write drm/amdgpu: EEPROM: add explicit read and write drm/amd/pm: Extend the I2C quirk table drm/amd/pm: Simplify managed I2C transfer functions drm/amdgpu: Fix width of I2C address drm/amdgpu: Return result fix in RAS drm/amdgpu: Fix amdgpu_ras_eeprom_init() drm/amdgpu: Simplify RAS EEPROM checksum calculations drm/amdgpu: Use explicit cardinality for clarity drm/amdgpu: Optimizations to EEPROM RAS table I/O drm/amdgpu: RAS EEPROM table is now in debugfs drm/amdgpu: Fix koops when accessing RAS EEPROM drm/amdgpu: Use a single loop drm/amdgpu: Correctly disable the I2C IP block drivers/gpu/drm/amd/amdgpu/Makefile |3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |9 +- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c| 239 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h| 37 + .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c| 32 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 114 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h |1 + .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 1253 +++-- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h| 68 +- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c| 319 +++-- drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h |1 + .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 238 +--- .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 118 ++ .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 241 +--- 14 files changed, 1682 insertions(+), 991 deletions(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Guchun Chen Cc: Hawking Zhang Cc: Jean Delvare Cc: John Clements Cc: Lijo Lazar Cc: Stanley Yang Cc: Xinhui Pan base-commit: a6e6e5fdc06791ebf5dbcd1ac64d555fe23f30e5 -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 29/40] drm/amd/pm: Extend the I2C quirk table
Extend the I2C quirk table for SMU access controlled I2C adapters. Let the kernel I2C layer check that the messages all have the same address, and that their combined size doesn't exceed the maximum size of a SMU software I2C request. Suggested-by: Jean Delvare Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 5 - drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 5 - drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 5 - 3 files changed, 12 insertions(+), 3 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 9fccdd2d3e73ec..3286c704bd08df 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -2036,8 +2036,11 @@ static const struct i2c_algorithm arcturus_i2c_algo = { static const struct i2c_adapter_quirks arcturus_i2c_control_quirks = { - .max_read_len = MAX_SW_I2C_COMMANDS, + .flags = I2C_AQ_COMB | I2C_AQ_COMB_SAME_ADDR, + .max_read_len = MAX_SW_I2C_COMMANDS, .max_write_len = MAX_SW_I2C_COMMANDS, + .max_comb_1st_msg_len = 2, + .max_comb_2nd_msg_len = MAX_SW_I2C_COMMANDS - 2, }; static int arcturus_i2c_control_init(struct smu_context *smu, struct i2c_adapter *control) 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 e3cbd334a956d1..e9c14ab7699502 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2834,8 +2834,11 @@ static const struct i2c_algorithm navi10_i2c_algo = { }; static const struct i2c_adapter_quirks navi10_i2c_control_quirks = { - .max_read_len = MAX_SW_I2C_COMMANDS, + .flags = I2C_AQ_COMB | I2C_AQ_COMB_SAME_ADDR, + .max_read_len = MAX_SW_I2C_COMMANDS, .max_write_len = MAX_SW_I2C_COMMANDS, + .max_comb_1st_msg_len = 2, + .max_comb_2nd_msg_len = MAX_SW_I2C_COMMANDS - 2, }; static int navi10_i2c_control_init(struct smu_context *smu, struct i2c_adapter *control) 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 2444e13c4901b3..37cfe0ccd6863b 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 @@ -3541,8 +3541,11 @@ static const struct i2c_algorithm sienna_cichlid_i2c_algo = { }; static const struct i2c_adapter_quirks sienna_cichlid_i2c_control_quirks = { - .max_read_len = MAX_SW_I2C_COMMANDS, + .flags = I2C_AQ_COMB | I2C_AQ_COMB_SAME_ADDR, + .max_read_len = MAX_SW_I2C_COMMANDS, .max_write_len = MAX_SW_I2C_COMMANDS, + .max_comb_1st_msg_len = 2, + .max_comb_2nd_msg_len = MAX_SW_I2C_COMMANDS - 2, }; static int sienna_cichlid_i2c_control_init(struct smu_context *smu, struct i2c_adapter *control) -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 23/40] drm/amdgpu: Fix wrap-around bugs in RAS
Fix the size of the EEPROM from 256000 bytes to 262144 bytes (256 KiB). Fix a couple or wrap around bugs. If a valid value/address is 0 <= addr < size, the inverse of this inequality (barring negative values which make no sense here) is addr >= size. Fix this in the RAS code. Cc: Jean Delvare Cc: Alexander Deucher Cc: Andrey Grodzovsky Cc: Lijo Lazar Cc: Stanley Yang Cc: Hawking Zhang Signed-off-by: Luben Tuikov Acked-by: Alexander Deucher --- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 20 +-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index f316fb11b16d9e..3ef38b90fc3a83 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -52,12 +52,11 @@ /* Bad GPU tag ‘BADG’ */ #define EEPROM_TABLE_HDR_BAD 0x42414447 -/* Assume 2 Mbit size */ -#define EEPROM_SIZE_BYTES 256000 -#define EEPROM_PAGE__SIZE_BYTES 256 -#define EEPROM_HDR_START 0 -#define EEPROM_RECORD_START (EEPROM_HDR_START + EEPROM_TABLE_HEADER_SIZE) -#define EEPROM_MAX_RECORD_NUM ((EEPROM_SIZE_BYTES - EEPROM_TABLE_HEADER_SIZE) / EEPROM_TABLE_RECORD_SIZE) +/* Assume 2-Mbit size */ +#define EEPROM_SIZE_BYTES (256 * 1024) +#define EEPROM_HDR_START0 +#define EEPROM_RECORD_START (EEPROM_HDR_START + EEPROM_TABLE_HEADER_SIZE) +#define EEPROM_MAX_RECORD_NUM ((EEPROM_SIZE_BYTES - EEPROM_TABLE_HEADER_SIZE) / EEPROM_TABLE_RECORD_SIZE) #define to_amdgpu_device(x) (container_of(x, struct amdgpu_ras, eeprom_control))->adev @@ -402,9 +401,8 @@ static uint32_t __correct_eeprom_dest_address(uint32_t curr_address) uint32_t next_address = curr_address + EEPROM_TABLE_RECORD_SIZE; /* When all EEPROM memory used jump back to 0 address */ - if (next_address > EEPROM_SIZE_BYTES) { - DRM_INFO("Reached end of EEPROM memory, jumping to 0 " -"and overriding old record"); + if (next_address >= EEPROM_SIZE_BYTES) { + DRM_INFO("Reached end of EEPROM memory, wrap around to 0."); return EEPROM_RECORD_START; } @@ -476,7 +474,9 @@ int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, } /* In case of overflow just start from beginning to not lose newest records */ - if (write && (control->next_addr + EEPROM_TABLE_RECORD_SIZE * num > EEPROM_SIZE_BYTES)) + if (write && + (control->next_addr + +EEPROM_TABLE_RECORD_SIZE * num >= EEPROM_SIZE_BYTES)) control->next_addr = EEPROM_RECORD_START; /* -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 05/40] drm/amdgpu: add new helper for handling EEPROM i2c transfers
From: Alex Deucher Encapsulates the i2c protocol handling so other parts of the driver can just tell it the offset and size of data to write. Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/Makefile| 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 67 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h | 34 +++ 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index c56320e78c0e1f..7d292485ca7cf2 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -57,7 +57,8 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o amdgpu_vm_cpu.o \ amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \ amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \ - amdgpu_fw_attestation.o amdgpu_securedisplay.o amdgpu_hdp.o + amdgpu_fw_attestation.o amdgpu_securedisplay.o amdgpu_hdp.o \ + amdgpu_eeprom.o amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c new file mode 100644 index 00..10551660343278 --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c @@ -0,0 +1,67 @@ +/* + * Copyright 2021 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +#include "amdgpu_eeprom.h" +#include "amdgpu.h" + +#define EEPROM_OFFSET_LENGTH 2 + +int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, + u16 slave_addr, u16 eeprom_addr, + u8 *eeprom_buf, u16 bytes, bool read) +{ + u8 eeprom_offset_buf[2]; + u16 bytes_transferred; + struct i2c_msg msgs[] = { + { + .addr = slave_addr, + .flags = 0, + .len = EEPROM_OFFSET_LENGTH, + .buf = eeprom_offset_buf, + }, + { + .addr = slave_addr, + .flags = read ? I2C_M_RD: 0, + .len = bytes, + .buf = eeprom_buf, + } + }; + int r; + + msgs[0].buf[0] = ((eeprom_addr >> 8) & 0xff); + msgs[0].buf[1] = (eeprom_addr & 0xff); + + while (msgs[1].len) { + r = i2c_transfer(i2c_adap, msgs, ARRAY_SIZE(msgs)); + if (r <= 0) + return r; + bytes_transferred = r - EEPROM_OFFSET_LENGTH; + eeprom_addr += bytes_transferred; + msgs[0].buf[0] = ((eeprom_addr >> 8) & 0xff); + msgs[0].buf[1] = (eeprom_addr & 0xff); + msgs[1].buf += bytes_transferred; + msgs[1].len -= bytes_transferred; + } + + return 0; +} diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h new file mode 100644 index 00..9301e5678910ad --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h @@ -0,0 +1,34 @@ +/* + * Copyright 2021 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of
[PATCH 04/40] drm/amdgpu/pm: add smu i2c implementation for navi1x (v4)
From: Alex Deucher And handle more than just EEPROMs. v2: fix restart handling between transactions. v3: handle 7 to 8 bit addr conversion v4: Fix --> req. (Luben T) Signed-off-by: Alex Deucher Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov --- .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 116 ++ 1 file changed, 116 insertions(+) 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 0e67517682e319..f426f488c02561 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2735,6 +2735,120 @@ static ssize_t navi10_get_legacy_gpu_metrics(struct smu_context *smu, return sizeof(struct gpu_metrics_v1_3); } +static int navi10_i2c_xfer(struct i2c_adapter *i2c_adap, + struct i2c_msg *msgs, int num) +{ + struct amdgpu_device *adev = to_amdgpu_device(i2c_adap); + struct smu_table_context *smu_table = >smu.smu_table; + struct smu_table *table = _table->driver_table; + SwI2cRequest_t *req, *res = (SwI2cRequest_t *)table->cpu_addr; + u16 bytes_to_transfer, remaining_bytes, msg_bytes; + u16 available_bytes = MAX_SW_I2C_COMMANDS; + int i, j, r, c; + 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; + bytes_to_transfer += min(msgs[i].len, available_bytes); + available_bytes -= bytes_to_transfer; + } + + req = kzalloc(sizeof(*req), GFP_KERNEL); + if (!req) + return -ENOMEM; + + req->I2CcontrollerPort = 1; + req->I2CSpeed = I2C_SPEED_FAST_400K; + req->SlaveAddress = slave << 1; /* 8 bit addresses */ + req->NumCmds = bytes_to_transfer; + + remaining_bytes = bytes_to_transfer; + c = 0; + for (i = 0; i < num; i++) { + struct i2c_msg *msg = [i]; + + msg_bytes = min(msg->len, remaining_bytes); + for (j = 0; j < msg_bytes; j++) { + SwI2cCmd_t *cmd = >SwI2cCmds[c++]; + + remaining_bytes--; + if (!(msg[i].flags & I2C_M_RD)) { + /* write */ + cmd->CmdConfig |= I2C_CMD_WRITE; + cmd->RegisterAddr = msg->buf[j]; + } + if ((msg[i].flags & I2C_M_STOP) || + (!remaining_bytes)) + cmd->CmdConfig |= CMDCONFIG_STOP_MASK; + if ((i > 0) && !(msg[i].flags & I2C_M_NOSTART)) + cmd->CmdConfig |= CMDCONFIG_RESTART_BIT; + } + } + mutex_lock(>smu.mutex); + r = smu_cmn_update_table(>smu, SMU_TABLE_I2C_COMMANDS, 0, req, true); + mutex_unlock(>smu.mutex); + if (r) + goto fail; + + remaining_bytes = bytes_to_transfer; + c = 0; + for (i = 0; i < num; i++) { + struct i2c_msg *msg = [i]; + + msg_bytes = min(msg->len, remaining_bytes); + for (j = 0; j < msg_bytes; j++) { + SwI2cCmd_t *cmd = >SwI2cCmds[c++]; + + remaining_bytes--; + if (msg[i].flags & I2C_M_RD) + msg->buf[j] = cmd->Data; + } + } + r = bytes_to_transfer; + +fail: + kfree(req); + + return r; +} + +static u32 navi10_i2c_func(struct i2c_adapter *adap) +{ + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; +} + + +static const struct i2c_algorithm navi10_i2c_algo = { + .master_xfer = navi10_i2c_xfer, + .functionality = navi10_i2c_func, +}; + +static int navi10_i2c_control_init(struct smu_context *smu, 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 = >pdev->dev; + control->algo = _i2c_algo; + snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); + + res = i2c_add_adapter(control); + if (res) + DRM_ERROR("Failed to register hw i2c, err: %d\n", res); + + return res; +} + +static void navi10_i2c_control_fini(struct smu_context *smu, struct i2c_adapter *control) +{ + i2c_del_adapter(control); +} + static ssize_t navi10_get_gpu_metrics(struct smu_context *smu, void **table) { @@ -3078,6 +3192,8 @@ static const struct pptable_funcs navi10_ppt_funcs = { .set_default_dpm_table = navi10_set_default_dpm_table, .dpm_set_vcn_enable = navi10_dpm_set_vcn_enable, .dpm_set_jpeg_enable =
[PATCH 10/40] drm/amdgpu: rework smu11 i2c for generic operation
From: Aaron Rice Handle things besides EEPROMS. Signed-off-by: Aaron Rice Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 47 +- 1 file changed, 9 insertions(+), 38 deletions(-) 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 3a164d93c90293..3193d566f4f87e 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -117,8 +117,7 @@ static void smu_v11_0_i2c_set_address(struct i2c_adapter *control, uint8_t addre { struct amdgpu_device *adev = to_amdgpu_device(control); - /* Convert fromr 8-bit to 7-bit address */ - address >>= 1; + /* We take 7-bit addresses raw */ WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_TAR, (address & 0xFF)); } @@ -531,22 +530,14 @@ static bool smu_v11_0_i2c_bus_unlock(struct i2c_adapter *control) /* I2C GLUE / static uint32_t smu_v11_0_i2c_read_data(struct i2c_adapter *control, - uint8_t address, - uint8_t *data, - uint32_t numbytes) + struct i2c_msg *msg) { uint32_t ret = 0; - /* First 2 bytes are dummy write to set EEPROM address */ - ret = smu_v11_0_i2c_transmit(control, address, data, 2, I2C_NO_STOP); - if (ret != I2C_OK) - goto Fail; - /* Now read data starting with that address */ - ret = smu_v11_0_i2c_receive(control, address, data + 2, numbytes - 2, + ret = smu_v11_0_i2c_receive(control, msg->addr, msg->buf, msg->len, I2C_RESTART); -Fail: if (ret != I2C_OK) DRM_ERROR("ReadData() - I2C error occurred :%x", ret); @@ -554,28 +545,16 @@ static uint32_t smu_v11_0_i2c_read_data(struct i2c_adapter *control, } static uint32_t smu_v11_0_i2c_write_data(struct i2c_adapter *control, -uint8_t address, -uint8_t *data, -uint32_t numbytes) + struct i2c_msg *msg) { uint32_t ret; - ret = smu_v11_0_i2c_transmit(control, address, data, numbytes, 0); + /* Send I2C_NO_STOP unless requested to stop. */ + ret = smu_v11_0_i2c_transmit(control, msg->addr, msg->buf, msg->len, ((msg->flags & I2C_M_STOP) ? 0 : I2C_NO_STOP)); if (ret != I2C_OK) DRM_ERROR("WriteI2CData() - I2C error occurred :%x", ret); - else - /* -* 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. -* -* TODO Improve to wait for first ACK for slave address after -* internal write cycle done. -*/ - msleep(10); - + return ret; } @@ -618,24 +597,16 @@ static int smu_v11_0_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg *msgs, int num) { int i, ret; - struct amdgpu_device *adev = to_amdgpu_device(i2c_adap); - - if (!adev->pm.bus_locked) { - DRM_ERROR("I2C bus unlocked, stopping transaction!"); - return -EIO; - } smu_v11_0_i2c_init(i2c_adap); for (i = 0; i < num; i++) { if (msgs[i].flags & I2C_M_RD) ret = smu_v11_0_i2c_read_data(i2c_adap, - (uint8_t)msgs[i].addr, - msgs[i].buf, msgs[i].len); + msgs + i); else ret = smu_v11_0_i2c_write_data(i2c_adap, - (uint8_t)msgs[i].addr, - msgs[i].buf, msgs[i].len); + msgs + i); if (ret != I2C_OK) { num = -EIO; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 07/40] drm/amdgpu/ras: switch fru eeprom handling to use generic helper (v2)
From: Alex Deucher Use the new helper rather than doing i2c transfers directly. v2: fix typo Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c| 22 +-- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c index 39b6c6bfab4533..224da573ba1b59 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c @@ -27,9 +27,9 @@ #include "smu_v11_0_i2c.h" #include "atom.h" #include "amdgpu_fru_eeprom.h" +#include "amdgpu_eeprom.h" #define I2C_PRODUCT_INFO_ADDR 0xAC -#define I2C_PRODUCT_INFO_ADDR_SIZE 0x2 #define I2C_PRODUCT_INFO_OFFSET0xC0 static bool is_fru_eeprom_supported(struct amdgpu_device *adev) @@ -65,16 +65,9 @@ static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr, unsigned char *buff) { int ret, size; - struct i2c_msg msg = { - .addr = I2C_PRODUCT_INFO_ADDR, - .flags = I2C_M_RD, - .buf= buff, - }; - buff[0] = 0; - buff[1] = addrptr; - msg.len = I2C_PRODUCT_INFO_ADDR_SIZE + 1; - ret = i2c_transfer(>pm.smu_i2c, , 1); + ret = amdgpu_eeprom_xfer(>pm.smu_i2c, I2C_PRODUCT_INFO_ADDR, +addrptr, buff, 1, true); if (ret < 1) { DRM_WARN("FRU: Failed to get size field"); return ret; @@ -83,13 +76,10 @@ static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr, /* The size returned by the i2c requires subtraction of 0xC0 since the * size apparently always reports as 0xC0+actual size. */ - size = buff[2] - I2C_PRODUCT_INFO_OFFSET; - /* Add 1 since address field was 1 byte */ - buff[1] = addrptr + 1; - - msg.len = I2C_PRODUCT_INFO_ADDR_SIZE + size; - ret = i2c_transfer(>pm.smu_i2c, , 1); + size = buff[0] - I2C_PRODUCT_INFO_OFFSET; + ret = amdgpu_eeprom_xfer(>pm.smu_i2c, I2C_PRODUCT_INFO_ADDR, +addrptr + 1, buff, size, true); if (ret < 1) { DRM_WARN("FRU: Failed to get data field"); return ret; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 06/40] drm/amdgpu/ras: switch ras eeprom handling to use generic helper
From: Alex Deucher Use the new helper rather than doing i2c transfers directly. Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 86 ++- 1 file changed, 28 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c index f40c871da0c623..e22a0b45f70108 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -26,6 +26,7 @@ #include "amdgpu_ras.h" #include #include "atom.h" +#include "amdgpu_eeprom.h" #define EEPROM_I2C_TARGET_ADDR_VEGA20 0xA0 #define EEPROM_I2C_TARGET_ADDR_ARCTURUS0xA8 @@ -148,22 +149,13 @@ static int __update_table_header(struct amdgpu_ras_eeprom_control *control, { int ret = 0; struct amdgpu_device *adev = to_amdgpu_device(control); - struct i2c_msg msg = { - .addr = 0, - .flags = 0, - .len= EEPROM_ADDRESS_SIZE + EEPROM_TABLE_HEADER_SIZE, - .buf= buff, - }; - - *(uint16_t *)buff = EEPROM_HDR_START; - __encode_table_header_to_buff(>tbl_hdr, buff + EEPROM_ADDRESS_SIZE); - - msg.addr = control->i2c_address; + __encode_table_header_to_buff(>tbl_hdr, buff); /* i2c may be unstable in gpu reset */ down_read(>reset_sem); - ret = i2c_transfer(>pm.smu_i2c, , 1); + ret = amdgpu_eeprom_xfer(>pm.smu_i2c, control->i2c_address, +EEPROM_HDR_START, buff, EEPROM_TABLE_HEADER_SIZE, false); up_read(>reset_sem); if (ret < 1) @@ -289,15 +281,9 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control, { int ret = 0; struct amdgpu_device *adev = to_amdgpu_device(control); - unsigned char buff[EEPROM_ADDRESS_SIZE + EEPROM_TABLE_HEADER_SIZE] = { 0 }; + unsigned char buff[EEPROM_TABLE_HEADER_SIZE] = { 0 }; struct amdgpu_ras_eeprom_table_header *hdr = >tbl_hdr; struct amdgpu_ras *ras = amdgpu_ras_get_context(adev); - struct i2c_msg msg = { - .addr = 0, - .flags = I2C_M_RD, - .len= EEPROM_ADDRESS_SIZE + EEPROM_TABLE_HEADER_SIZE, - .buf= buff, - }; *exceed_err_limit = false; @@ -313,9 +299,9 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control, mutex_init(>tbl_mutex); - msg.addr = control->i2c_address; /* Read/Create table header from EEPROM address 0 */ - ret = i2c_transfer(>pm.smu_i2c, , 1); + ret = amdgpu_eeprom_xfer(>pm.smu_i2c, control->i2c_address, +EEPROM_HDR_START, buff, EEPROM_TABLE_HEADER_SIZE, true); if (ret < 1) { DRM_ERROR("Failed to read EEPROM table header, ret:%d", ret); return ret; @@ -442,6 +428,7 @@ static uint32_t __correct_eeprom_dest_address(uint32_t curr_address) bool amdgpu_ras_eeprom_check_err_threshold(struct amdgpu_device *adev) { + struct amdgpu_ras *con = amdgpu_ras_get_context(adev); if (!__is_ras_eeprom_supported(adev)) @@ -470,11 +457,11 @@ int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, int num) { int i, ret = 0; - struct i2c_msg *msgs, *msg; unsigned char *buffs, *buff; struct eeprom_table_record *record; struct amdgpu_device *adev = to_amdgpu_device(control); struct amdgpu_ras *ras = amdgpu_ras_get_context(adev); + u16 slave_addr; if (!__is_ras_eeprom_supported(adev)) return 0; @@ -486,12 +473,6 @@ int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, mutex_lock(>tbl_mutex); - msgs = kcalloc(num, sizeof(*msgs), GFP_KERNEL); - if (!msgs) { - ret = -ENOMEM; - goto free_buff; - } - /* * If saved bad pages number exceeds the bad page threshold for * the whole VRAM, update table header to mark the BAD GPU tag @@ -521,9 +502,8 @@ int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, * 256b */ for (i = 0; i < num; i++) { - buff = [i * (EEPROM_ADDRESS_SIZE + EEPROM_TABLE_RECORD_SIZE)]; + buff = [i * EEPROM_TABLE_RECORD_SIZE]; record = [i]; - msg = [i]; control->next_addr = __correct_eeprom_dest_address(control->next_addr); @@ -531,20 +511,26 @@ int amdgpu_ras_eeprom_process_recods(struct amdgpu_ras_eeprom_control *control, * Update bits 16,17 of EEPROM address in I2C address by setting them * to bits 1,2 of Device address byte */
[PATCH 02/40] drm/amdgpu/pm: rework i2c xfers on sienna cichlid (v4)
From: Alex Deucher Make it generic so we can support more than just EEPROMs. v2: fix restart handling between transactions. v3: handle 7 to 8 bit addr conversion v4: Fix --> req. (Luben T) Signed-off-by: Alex Deucher Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov --- .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 229 +- 1 file changed, 58 insertions(+), 171 deletions(-) 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 c751f717a0daff..b4aa489e4a431a 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 @@ -3442,197 +3442,84 @@ static void sienna_cichlid_dump_pptable(struct smu_context *smu) dev_info(smu->adev->dev, "MmHubPadding[7] = 0x%x\n", pptable->MmHubPadding[7]); } -static void sienna_cichlid_fill_i2c_req(SwI2cRequest_t *req, bool write, - uint8_t address, uint32_t numbytes, - uint8_t *data) -{ - int i; - - req->I2CcontrollerPort = 1; - req->I2CSpeed = 2; - req->SlaveAddress = address; - req->NumCmds = numbytes; - - for (i = 0; i < numbytes; i++) { - SwI2cCmd_t *cmd = >SwI2cCmds[i]; - - /* First 2 bytes are always write for lower 2b EEPROM address */ - if (i < 2) - cmd->CmdConfig = CMDCONFIG_READWRITE_MASK; - else - cmd->CmdConfig = write ? CMDCONFIG_READWRITE_MASK : 0; - - - /* 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->ReadWriteData = data[i]; - } -} - -static int sienna_cichlid_i2c_read_data(struct i2c_adapter *control, - uint8_t address, - uint8_t *data, - uint32_t numbytes) +static int sienna_cichlid_i2c_xfer(struct i2c_adapter *i2c_adap, + struct i2c_msg *msgs, int num) { - uint32_t i, ret = 0; - SwI2cRequest_t req; - struct amdgpu_device *adev = to_amdgpu_device(control); + struct amdgpu_device *adev = to_amdgpu_device(i2c_adap); struct smu_table_context *smu_table = >smu.smu_table; struct smu_table *table = _table->driver_table; - - if (numbytes > MAX_SW_I2C_COMMANDS) { - dev_err(adev->dev, "numbytes requested %d is over max allowed %d\n", - numbytes, MAX_SW_I2C_COMMANDS); - return -EINVAL; + SwI2cRequest_t *req, *res = (SwI2cRequest_t *)table->cpu_addr; + u16 bytes_to_transfer, remaining_bytes, msg_bytes; + u16 available_bytes = MAX_SW_I2C_COMMANDS; + int i, j, r, c; + 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; + bytes_to_transfer += min(msgs[i].len, available_bytes); + available_bytes -= bytes_to_transfer; } - memset(, 0, sizeof(req)); - sienna_cichlid_fill_i2c_req(, false, address, numbytes, data); - - mutex_lock(>smu.mutex); - /* Now read data starting with that address */ - ret = smu_cmn_update_table(>smu, SMU_TABLE_I2C_COMMANDS, 0, , - true); - mutex_unlock(>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].ReadWriteData; - - dev_dbg(adev->dev, "sienna_cichlid_i2c_read_data, address = %x, bytes = %d, data :", - (uint16_t)address, numbytes); + req = kzalloc(sizeof(*req), GFP_KERNEL); + if (!req) + return -ENOMEM; - print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_NONE, - 8, 1, data, numbytes, false); - } else - dev_err(adev->dev, "sienna_cichlid_i2c_read_data - error occurred :%x", ret); + req->I2CcontrollerPort = 1; + req->I2CSpeed = I2C_SPEED_FAST_400K; + req->SlaveAddress = slave << 1; /* 8 bit addresses */ + req->NumCmds = bytes_to_transfer; - return ret; -} + remaining_bytes = bytes_to_transfer; + c = 0; + for (i = 0; i < num; i++) { +
[PATCH 09/40] drm/amdgpu: add I2C_CLASS_HWMON to SMU i2c buses
From: Alex Deucher Not sure that this really matters that much, but these could have various other hwmon chips on them. Signed-off-by: Alex Deucher Reviewed-by: Luben Tuikov --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) 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 1d8f6d5180e099..3a164d93c90293 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -665,7 +665,7 @@ int smu_v11_0_i2c_control_init(struct i2c_adapter *control) mutex_init(>pm.smu_i2c_mutex); control->owner = THIS_MODULE; - control->class = I2C_CLASS_SPD; + control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _v11_0_i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); 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 a6447822deb09f..404afc9979c69b 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c @@ -2033,7 +2033,7 @@ static int arcturus_i2c_control_init(struct smu_context *smu, struct i2c_adapter int res; control->owner = THIS_MODULE; - control->class = I2C_CLASS_SPD; + control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); 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 f426f488c02561..d3efe897ebbb2f 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2832,7 +2832,7 @@ static int navi10_i2c_control_init(struct smu_context *smu, struct i2c_adapter * int res; control->owner = THIS_MODULE; - control->class = I2C_CLASS_SPD; + control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); 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 b4aa489e4a431a..1d06641ad87890 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 @@ -3539,7 +3539,7 @@ static int sienna_cichlid_i2c_control_init(struct smu_context *smu, struct i2c_a int res; control->owner = THIS_MODULE; - control->class = I2C_CLASS_SPD; + control->class = I2C_CLASS_SPD | I2C_CLASS_HWMON; control->dev.parent = >pdev->dev; control->algo = _cichlid_i2c_algo; snprintf(control->name, sizeof(control->name), "AMDGPU SMU"); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] drm/amdgpu: parameterize ttm BO destroy callback
On 6/14/2021 7:10 PM, Christian König wrote: Am 14.06.21 um 16:32 schrieb Nirmoy Das: Make provision to pass different ttm BO destroy callback while creating a amdgpu_bo. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++ 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 9092ac12a270..71a65525eac4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -522,15 +522,17 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) * @adev: amdgpu device object * @bp: parameters to be used for the buffer object * @bo_ptr: pointer to the buffer object pointer + * @destroy: ttm bo destroy callback * - * Creates an _bo buffer object. + * Creates an _bo buffer object with a specified ttm bo destroy callback. * * Returns: * 0 for success or a negative error code on failure. */ -int amdgpu_bo_create(struct amdgpu_device *adev, +static int amdgpu_bo_do_create(struct amdgpu_device *adev, Please don't. Rather expose an amdgpu_vm_bo_create function. struct amdgpu_bo_param *bp, - struct amdgpu_bo **bo_ptr) + struct amdgpu_bo **bo_ptr, + void (*destroy)(struct ttm_buffer_object *)) That rather belongs into the amdgpu_bo_param structure. I misinterpret Teams conversation. I will resend. Thanks, Nirmoy { struct ttm_operation_ctx ctx = { .interruptible = (bp->type != ttm_bo_type_kernel), @@ -594,7 +596,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type, >placement, page_align, , NULL, - bp->resv, _bo_destroy); + bp->resv, destroy); if (unlikely(r != 0)) return r; @@ -638,6 +640,24 @@ int amdgpu_bo_create(struct amdgpu_device *adev, return r; } +/** + * amdgpu_bo_create - create an _bo buffer object + * @adev: amdgpu device object + * @bp: parameters to be used for the buffer object + * @bo_ptr: pointer to the buffer object pointer + * + * Creates an _bo buffer object. + * + * Returns: + * 0 for success or a negative error code on failure. + */ +int amdgpu_bo_create(struct amdgpu_device *adev, + struct amdgpu_bo_param *bp, + struct amdgpu_bo **bo_ptr) +{ + return amdgpu_bo_do_create(adev, bp, bo_ptr, _bo_destroy); +} + /** * amdgpu_bo_create_user - create an _bo_user buffer object * @adev: amdgpu device object ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] drm/amdgpu: parameterize ttm BO destroy callback
Am 14.06.21 um 16:32 schrieb Nirmoy Das: Make provision to pass different ttm BO destroy callback while creating a amdgpu_bo. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++ 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 9092ac12a270..71a65525eac4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -522,15 +522,17 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) * @adev: amdgpu device object * @bp: parameters to be used for the buffer object * @bo_ptr: pointer to the buffer object pointer + * @destroy: ttm bo destroy callback * - * Creates an _bo buffer object. + * Creates an _bo buffer object with a specified ttm bo destroy callback. * * Returns: * 0 for success or a negative error code on failure. */ -int amdgpu_bo_create(struct amdgpu_device *adev, +static int amdgpu_bo_do_create(struct amdgpu_device *adev, Please don't. Rather expose an amdgpu_vm_bo_create function. struct amdgpu_bo_param *bp, - struct amdgpu_bo **bo_ptr) + struct amdgpu_bo **bo_ptr, + void (*destroy)(struct ttm_buffer_object *)) That rather belongs into the amdgpu_bo_param structure. { struct ttm_operation_ctx ctx = { .interruptible = (bp->type != ttm_bo_type_kernel), @@ -594,7 +596,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type, >placement, page_align, , NULL, -bp->resv, _bo_destroy); +bp->resv, destroy); if (unlikely(r != 0)) return r; @@ -638,6 +640,24 @@ int amdgpu_bo_create(struct amdgpu_device *adev, return r; } +/** + * amdgpu_bo_create - create an _bo buffer object + * @adev: amdgpu device object + * @bp: parameters to be used for the buffer object + * @bo_ptr: pointer to the buffer object pointer + * + * Creates an _bo buffer object. + * + * Returns: + * 0 for success or a negative error code on failure. + */ +int amdgpu_bo_create(struct amdgpu_device *adev, + struct amdgpu_bo_param *bp, + struct amdgpu_bo **bo_ptr) +{ + return amdgpu_bo_do_create(adev, bp, bo_ptr, _bo_destroy); +} + /** * amdgpu_bo_create_user - create an _bo_user buffer object * @adev: amdgpu device object ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
On Mon, Jun 14, 2021 at 3:27 PM Nirmoy Das wrote: > > Make provision to pass different ttm BO destroy callback > while creating a amdgpu_bo. > > v2: pass destroy callback using amdgpu_bo_param. > > Signed-off-by: Nirmoy Das > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 3 +- > 2 files changed, 42 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index 9092ac12a270..f4f57a73d095 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -12,7 +12,7 @@ > * > * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL > + * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL > * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY > CLAIM, > * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE Unrelated whitespace change. Please drop. Alex > @@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo > *bo) > } > } > > -static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo) > +static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo) > { > - struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); > struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); > - struct amdgpu_bo_user *ubo; > > if (bo->tbo.pin_count > 0) > amdgpu_bo_subtract_pin_size(bo); > @@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object > *tbo) > if (bo->tbo.base.import_attach) > drm_prime_gem_destroy(>tbo.base, bo->tbo.sg); > drm_gem_object_release(>tbo.base); > + amdgpu_bo_unref(>parent); > +} > + > +static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo) > +{ > + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); > + > + amdgpu_bo_destroy_base(tbo); > + kvfree(bo); > +} > + > +static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo) > +{ > + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); > + struct amdgpu_bo_user *ubo; > + > + amdgpu_bo_destroy_base(tbo); > + ubo = to_amdgpu_bo_user(bo); > + kfree(ubo->metadata); > + kvfree(bo); > +} > + > +static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo) > +{ > + struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); > + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); > + > + amdgpu_bo_destroy_base(tbo); > /* in case amdgpu_device_recover_vram got NULL of bo->parent */ > if (!list_empty(>shadow_list)) { > mutex_lock(>shadow_list_lock); > list_del_init(>shadow_list); > mutex_unlock(>shadow_list_lock); > } > - amdgpu_bo_unref(>parent); > - > - if (bo->tbo.type != ttm_bo_type_kernel) { > - ubo = to_amdgpu_bo_user(bo); > - kfree(ubo->metadata); > - } > > kvfree(bo); > } > @@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object > *tbo) > */ > bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo) > { > - if (bo->destroy == _bo_destroy) > + if (bo->destroy == _bo_destroy || > + bo->destroy == _bo_user_destroy || > + bo->destroy == _bo_vm_destroy) > return true; > + > return false; > } > > @@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev, > if (bp->type == ttm_bo_type_kernel) > bo->tbo.priority = 1; > > + if (!bp->destroy) > + bp->destroy = _bo_destroy; > + > r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type, > >placement, page_align, , NULL, > -bp->resv, _bo_destroy); > +bp->resv, bp->destroy); > if (unlikely(r != 0)) > return r; > > @@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev, > int r; > > bp->bo_ptr_size = sizeof(struct amdgpu_bo_user); > + bp->destroy = _bo_user_destroy; > r = amdgpu_bo_create(adev, bp, _ptr); > if (r) > return r; > @@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev, > * num of amdgpu_vm_pt entries. > */ > BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm)); > + bp->destroy = _bo_vm_destroy; > r = amdgpu_bo_create(adev, bp, _ptr); > if (r) > return r; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h >
Re: [RFC PATCH v2 1/8] ext4/xfs: add page refcount helper
Am 2021-06-09 um 3:23 p.m. schrieb Matthew Wilcox: > On Mon, Jun 07, 2021 at 03:42:19PM -0500, Alex Sierra wrote: >> +++ b/include/linux/dax.h >> @@ -243,6 +243,16 @@ static inline bool dax_mapping(struct address_space >> *mapping) >> return mapping->host && IS_DAX(mapping->host); >> } >> >> +static inline bool dax_layout_is_idle_page(struct page *page) >> +{ >> +return page_ref_count(page) == 1; >> +} > We already have something called an idle page, and that's quite a > different thing from this. How about dax_page_unused() (it's a use > count, so once it's got down to its minimum value, it's unused)? > Hi Matthew, Thank you very much for your feedback. This patch looks straight-forward enough, but do we need the filesystem maintainers to review this as well? I guess we should CC the linux-ext4 and linux-xfs mailing lists in the next revision. Hi Ralph, Are you OK if we update your patch with this suggestion? Thanks, Felix ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] drm/amdgpu: parameterize ttm BO destroy callback
On 6/14/2021 4:32 PM, Nirmoy Das wrote: Make provision to pass different ttm BO destroy callback while creating a amdgpu_bo. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++ 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 9092ac12a270..71a65525eac4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -522,15 +522,17 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) copy-paste typo : Need to replace function header amdgpu_bo_create -> amdgpu_bo_do_create. * @adev: amdgpu device object * @bp: parameters to be used for the buffer object * @bo_ptr: pointer to the buffer object pointer + * @destroy: ttm bo destroy callback * - * Creates an _bo buffer object. + * Creates an _bo buffer object with a specified ttm bo destroy callback. * * Returns: * 0 for success or a negative error code on failure. */ -int amdgpu_bo_create(struct amdgpu_device *adev, +static int amdgpu_bo_do_create(struct amdgpu_device *adev, struct amdgpu_bo_param *bp, - struct amdgpu_bo **bo_ptr) + struct amdgpu_bo **bo_ptr, + void (*destroy)(struct ttm_buffer_object *)) { struct ttm_operation_ctx ctx = { .interruptible = (bp->type != ttm_bo_type_kernel), @@ -594,7 +596,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type, >placement, page_align, , NULL, -bp->resv, _bo_destroy); +bp->resv, destroy); if (unlikely(r != 0)) return r; @@ -638,6 +640,24 @@ int amdgpu_bo_create(struct amdgpu_device *adev, return r; } +/** + * amdgpu_bo_create - create an _bo buffer object + * @adev: amdgpu device object + * @bp: parameters to be used for the buffer object + * @bo_ptr: pointer to the buffer object pointer + * + * Creates an _bo buffer object. + * + * Returns: + * 0 for success or a negative error code on failure. + */ +int amdgpu_bo_create(struct amdgpu_device *adev, + struct amdgpu_bo_param *bp, + struct amdgpu_bo **bo_ptr) +{ + return amdgpu_bo_do_create(adev, bp, bo_ptr, _bo_destroy); +} + /** * amdgpu_bo_create_user - create an _bo_user buffer object * @adev: amdgpu device object ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/3] drm/amdgpu: use separate ttm destroy callback
Use different ttm destroy callback for different type of amdgpu BO. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 47 -- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 71a65525eac4..d97c20346a8c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo) } } -static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo) +static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo) { - struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); - struct amdgpu_bo_user *ubo; if (bo->tbo.pin_count > 0) amdgpu_bo_subtract_pin_size(bo); @@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo) if (bo->tbo.base.import_attach) drm_prime_gem_destroy(>tbo.base, bo->tbo.sg); drm_gem_object_release(>tbo.base); + amdgpu_bo_unref(>parent); +} + +static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo) +{ + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); + + amdgpu_bo_destroy_base(tbo); + kvfree(bo); +} + +static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo) +{ + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); + struct amdgpu_bo_user *ubo; + + amdgpu_bo_destroy_base(tbo); + ubo = to_amdgpu_bo_user(bo); + kfree(ubo->metadata); + kvfree(bo); +} + +static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo) +{ + struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); + + amdgpu_bo_destroy_base(tbo); /* in case amdgpu_device_recover_vram got NULL of bo->parent */ if (!list_empty(>shadow_list)) { mutex_lock(>shadow_list_lock); list_del_init(>shadow_list); mutex_unlock(>shadow_list_lock); } - amdgpu_bo_unref(>parent); - - if (bo->tbo.type != ttm_bo_type_kernel) { - ubo = to_amdgpu_bo_user(bo); - kfree(ubo->metadata); - } kvfree(bo); } @@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo) */ bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo) { - if (bo->destroy == _bo_destroy) + if (bo->destroy == _bo_destroy || + bo->destroy == _bo_user_destroy || + bo->destroy == _bo_vm_destroy) return true; + return false; } @@ -678,7 +701,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev, int r; bp->bo_ptr_size = sizeof(struct amdgpu_bo_user); - r = amdgpu_bo_create(adev, bp, _ptr); + r = amdgpu_bo_do_create(adev, bp, _ptr, _bo_user_destroy); if (r) return r; @@ -709,7 +732,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev, * num of amdgpu_vm_pt entries. */ BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm)); - r = amdgpu_bo_create(adev, bp, _ptr); + r = amdgpu_bo_do_create(adev, bp, _ptr, _bo_vm_destroy); if (r) return r; -- 2.31.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/3] drm/amdgpu: move shadow_list to amdgpu_bo_vm
Move shadow_list to struct amdgpu_bo_vm as shadow BOs are part of PT/PD BOs. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 6 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index f2636f411a25..3f51b142fc83 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -4063,6 +4063,7 @@ static int amdgpu_device_recover_vram(struct amdgpu_device *adev) { struct dma_fence *fence = NULL, *next = NULL; struct amdgpu_bo *shadow; + struct amdgpu_bo_vm *vmbo; long r = 1, tmo; if (amdgpu_sriov_runtime(adev)) @@ -4072,8 +4073,8 @@ static int amdgpu_device_recover_vram(struct amdgpu_device *adev) dev_info(adev->dev, "recover vram bo from shadow start\n"); mutex_lock(>shadow_list_lock); - list_for_each_entry(shadow, >shadow_list, shadow_list) { - + list_for_each_entry(vmbo, >shadow_list, shadow_list) { + shadow = >bo; /* No need to recover an evicted BO */ if (shadow->tbo.mem.mem_type != TTM_PL_TT || shadow->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET || diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index d97c20346a8c..cb543e8abe35 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -111,12 +111,13 @@ static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo) { struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); + struct amdgpu_bo_vm *vmbo = to_amdgpu_bo_vm(bo); amdgpu_bo_destroy_base(tbo); /* in case amdgpu_device_recover_vram got NULL of bo->parent */ - if (!list_empty(>shadow_list)) { + if (!list_empty(>shadow_list)) { mutex_lock(>shadow_list_lock); - list_del_init(>shadow_list); + list_del_init(>shadow_list); mutex_unlock(>shadow_list_lock); } @@ -594,7 +595,6 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, if (bo == NULL) return -ENOMEM; drm_gem_private_object_init(adev_to_drm(adev), >tbo.base, size); - INIT_LIST_HEAD(>shadow_list); bo->vm_bo = NULL; bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain : bp->domain; @@ -737,6 +737,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev, return r; *vmbo_ptr = to_amdgpu_bo_vm(bo_ptr); + INIT_LIST_HEAD(&(*vmbo_ptr)->shadow_list); return r; } @@ -781,12 +782,12 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo) * * Insert a BO to the shadow list. */ -void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo) +void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo) { - struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); + struct amdgpu_device *adev = amdgpu_ttm_adev(vmbo->bo.tbo.bdev); mutex_lock(>shadow_list_lock); - list_add_tail(>shadow_list, >shadow_list); + list_add_tail(>shadow_list, >shadow_list); mutex_unlock(>shadow_list_lock); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index e36b84516b4e..a08a34351f12 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -108,9 +108,6 @@ struct amdgpu_bo { #ifdef CONFIG_MMU_NOTIFIER struct mmu_interval_notifiernotifier; #endif - - struct list_headshadow_list; - struct kgd_mem *kfd_bo; }; @@ -126,6 +123,7 @@ struct amdgpu_bo_user { struct amdgpu_bo_vm { struct amdgpu_bobo; struct amdgpu_bo*shadow; + struct list_headshadow_list; struct amdgpu_vm_bo_baseentries[]; }; @@ -332,7 +330,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo); int amdgpu_bo_validate(struct amdgpu_bo *bo); void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem, uint64_t *gtt_mem, uint64_t *cpu_mem); -void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo); +void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo); int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow, struct dma_fence **fence); uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 4c4c56581780..812c225538a7 100644 ---
[PATCH 1/3] drm/amdgpu: parameterize ttm BO destroy callback
Make provision to pass different ttm BO destroy callback while creating a amdgpu_bo. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++ 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 9092ac12a270..71a65525eac4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -522,15 +522,17 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) * @adev: amdgpu device object * @bp: parameters to be used for the buffer object * @bo_ptr: pointer to the buffer object pointer + * @destroy: ttm bo destroy callback * - * Creates an _bo buffer object. + * Creates an _bo buffer object with a specified ttm bo destroy callback. * * Returns: * 0 for success or a negative error code on failure. */ -int amdgpu_bo_create(struct amdgpu_device *adev, +static int amdgpu_bo_do_create(struct amdgpu_device *adev, struct amdgpu_bo_param *bp, - struct amdgpu_bo **bo_ptr) + struct amdgpu_bo **bo_ptr, + void (*destroy)(struct ttm_buffer_object *)) { struct ttm_operation_ctx ctx = { .interruptible = (bp->type != ttm_bo_type_kernel), @@ -594,7 +596,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type, >placement, page_align, , NULL, -bp->resv, _bo_destroy); +bp->resv, destroy); if (unlikely(r != 0)) return r; @@ -638,6 +640,24 @@ int amdgpu_bo_create(struct amdgpu_device *adev, return r; } +/** + * amdgpu_bo_create - create an _bo buffer object + * @adev: amdgpu device object + * @bp: parameters to be used for the buffer object + * @bo_ptr: pointer to the buffer object pointer + * + * Creates an _bo buffer object. + * + * Returns: + * 0 for success or a negative error code on failure. + */ +int amdgpu_bo_create(struct amdgpu_device *adev, + struct amdgpu_bo_param *bp, + struct amdgpu_bo **bo_ptr) +{ + return amdgpu_bo_do_create(adev, bp, bo_ptr, _bo_destroy); +} + /** * amdgpu_bo_create_user - create an _bo_user buffer object * @adev: amdgpu device object -- 2.31.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdkfd: avoid double alloc by ioctl_alloc and svm
On 2021-06-11 12:51 a.m., Alex Sierra wrote: [Why] Avoid duplicated memory allocation for address ranges that have been already allocated by either ioctl_alloc_memory_of_gpu or SVM mechanisms first. [How] For SVM first allocations Check if the address range passed into ioctl memory alloc does not exist already in the kfd_process svms->objects interval tree. For ioctl_alloc_memory_of_gpu first allocations Look for the address range into the interval tree VA from the VM inside of each pdds used in a kfd_process. Signed-off-by: Alex Sierra --- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 11 + drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 63 ++-- 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c index 5788a4656fa1..0cfa685d9b8a 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -1259,6 +1259,17 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep, if (args->size == 0) return -EINVAL; +#if IS_ENABLED(CONFIG_HSA_AMD_SVM) + struct svm_range_list *svms = >svms; + should take svms lock + if (interval_tree_iter_first(>objects, + args->va_addr >> PAGE_SHIFT, + (args->va_addr + args->size - 1) >> PAGE_SHIFT)) { + pr_info("Address: 0x%llx already allocated by SVM\n", + args->va_addr); + return -EADDRINUSE; + } +#endif dev = kfd_device_by_id(args->gpu_id); if (!dev) return -EINVAL; diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index 0f18bd0dc64e..883a9659cf8e 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c @@ -2485,9 +2485,40 @@ int svm_range_list_init(struct kfd_process *p) return 0; } +/** + * svm_range_is_vm_bo_mapped - check if virtual address range mapped already + * @p: current kfd_process + * @start: range start address, in pages + * @last: range last address, in pages + * + * The purpose is to avoid virtual address ranges already allocated by + * traditional kfd_ioctl_alloc_memory_of_gpu ioctl. + * It looks for each pdd in the kfd_process. + * + * Context: Process context + * + * Return true only if range has been mapped + */ +static bool +svm_range_is_vm_bo_mapped(struct kfd_process *p, uint64_t start, uint64_t last) +{ + uint32_t i; + + for (i = 0; i < p->n_pdds; i++) { + struct amdgpu_vm *vm = drm_priv_to_vm(p->pdds[i]->drm_priv); + take vm root page table bo lock + if (vm && interval_tree_iter_first(>va, start, last)) { + pr_debug("Range [0x%llx 0x%llx] already mapped\n", start, last); + return true; + } + } + + return false; +} + /** * svm_range_is_valid - check if virtual address range is valid - * @mm: current process mm_struct + * @mm: current kfd_process * @start: range start address, in pages * @size: range size, in pages * @@ -2496,28 +2527,28 @@ int svm_range_list_init(struct kfd_process *p) * Context: Process context * * Return: - * true - valid svm range - * false - invalid svm range + * 0 - OK, otherwise error code */ -static bool -svm_range_is_valid(struct mm_struct *mm, uint64_t start, uint64_t size) +static int +svm_range_is_valid(struct kfd_process *p, uint64_t start, uint64_t size) svm_range_is_vm_bo_mapped returns bool, this should return bool, or change svm_range_is_vm_bo_mapped to return int error code. Regards, Philip { const unsigned long device_vma = VM_IO | VM_PFNMAP | VM_MIXEDMAP; struct vm_area_struct *vma; unsigned long end; + unsigned long start_unchg = start; start <<= PAGE_SHIFT; end = start + (size << PAGE_SHIFT); - do { - vma = find_vma(mm, start); + vma = find_vma(p->mm, start); if (!vma || start < vma->vm_start || (vma->vm_flags & device_vma)) - return false; + return -EFAULT; start = min(end, vma->vm_end); } while (start < end); - return true; + return svm_range_is_vm_bo_mapped(p, start_unchg, (end - 1) >> PAGE_SHIFT) ? + -EADDRINUSE:0; } /** @@ -2826,9 +2857,9 @@ svm_range_set_attr(struct kfd_process *p, uint64_t start, uint64_t size, svm_range_list_lock_and_flush_work(svms, mm); - if (!svm_range_is_valid(mm, start, size)) { - pr_debug("invalid range\n"); - r = -EFAULT; + r = svm_range_is_valid(p, start, size); + if (r) { + pr_debug("invalid range r=%d\n", r); mmap_write_unlock(mm); goto out; } @@ -2929,15 +2960,17 @@ svm_range_get_attr(struct kfd_process *p, uint64_t start, uint64_t size, uint32_t flags = 0x; int gpuidx; uint32_t i; + int r = 0; pr_debug("svms 0x%p [0x%llx 0x%llx] nattr 0x%x\n", >svms, start, start + size - 1, nattr); mmap_read_lock(mm); - if (!svm_range_is_valid(mm, start, size)) { - pr_debug("invalid range\n"); + r = svm_range_is_valid(p, start, size); + if (r) { + pr_debug("invalid range r=%d\n", r);
[PATCH 2/2] drm/amdgpu: rework dma_resv handling v3
Drop the workaround and instead implement a better solution. Basically we are now chaining all submissions using a dma_fence_chain container and adding them as exclusive fence to the dma_resv object. This way other drivers can still sync to the single exclusive fence while amdgpu only sync to fences from different processes. v3: add the shared fence first before the exclusive one Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 62 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 65 - drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 - 6 files changed, 55 insertions(+), 79 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h index a130e766cbdb..c905a4cfc173 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h @@ -34,6 +34,7 @@ struct amdgpu_fpriv; struct amdgpu_bo_list_entry { struct ttm_validate_buffer tv; struct amdgpu_bo_va *bo_va; + struct dma_fence_chain *chain; uint32_tpriority; struct page **user_pages; booluser_invalidated; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 9ce649a1a8d3..25655414e9c0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -572,6 +572,20 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, goto out; } + amdgpu_bo_list_for_each_entry(e, p->bo_list) { + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo); + + e->bo_va = amdgpu_vm_bo_find(vm, bo); + + if (bo->tbo.base.dma_buf && !amdgpu_bo_explicit_sync(bo)) { + e->chain = dma_fence_chain_alloc(); + if (!e->chain) { + r = -ENOMEM; + goto error_validate; + } + } + } + amdgpu_cs_get_threshold_for_moves(p->adev, >bytes_moved_threshold, >bytes_moved_vis_threshold); p->bytes_moved = 0; @@ -599,15 +613,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, gws = p->bo_list->gws_obj; oa = p->bo_list->oa_obj; - amdgpu_bo_list_for_each_entry(e, p->bo_list) { - struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo); - - /* Make sure we use the exclusive slot for shared BOs */ - if (bo->prime_shared_count) - e->tv.num_shared = 0; - e->bo_va = amdgpu_vm_bo_find(vm, bo); - } - if (gds) { p->job->gds_base = amdgpu_bo_gpu_offset(gds) >> PAGE_SHIFT; p->job->gds_size = amdgpu_bo_size(gds) >> PAGE_SHIFT; @@ -629,8 +634,13 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } error_validate: - if (r) + if (r) { + amdgpu_bo_list_for_each_entry(e, p->bo_list) { + dma_fence_chain_free(e->chain); + e->chain = NULL; + } ttm_eu_backoff_reservation(>ticket, >validated); + } out: return r; } @@ -670,9 +680,17 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, { unsigned i; - if (error && backoff) + if (error && backoff) { + struct amdgpu_bo_list_entry *e; + + amdgpu_bo_list_for_each_entry(e, parser->bo_list) { + dma_fence_chain_free(e->chain); + e->chain = NULL; + } + ttm_eu_backoff_reservation(>ticket, >validated); + } for (i = 0; i < parser->num_post_deps; i++) { drm_syncobj_put(parser->post_deps[i].syncobj); @@ -1245,6 +1263,28 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, amdgpu_vm_move_to_lru_tail(p->adev, >vm); + amdgpu_bo_list_for_each_entry(e, p->bo_list) { + struct dma_resv *resv = e->tv.bo->base.resv; + struct dma_fence_chain *chain = e->chain; + + if (!chain) + continue; + + /* +* Work around dma_resv shortcommings by wrapping up the +* submission in a dma_fence_chain and add it as exclusive +* fence, but first add the submission as shared fence to make +* sure that shared fences never signal before the exclusive +* one. +*/ +
[PATCH 1/2] drm/amdgpu: unwrap fence chains in the explicit sync fence
Unwrap the explicit fence if it is a dma_fence_chain and sync to the first fence not matching the owner rules. Signed-off-by: Christian König Acked-by: Daniel Vetter --- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 118 +-- 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c index 1b2ceccaf5b0..862eb3c1c4c5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c @@ -28,6 +28,8 @@ *Christian König */ +#include + #include "amdgpu.h" #include "amdgpu_trace.h" #include "amdgpu_amdkfd.h" @@ -186,6 +188,55 @@ int amdgpu_sync_vm_fence(struct amdgpu_sync *sync, struct dma_fence *fence) return amdgpu_sync_fence(sync, fence); } +/* Determine based on the owner and mode if we should sync to a fence or not */ +static bool amdgpu_sync_test_fence(struct amdgpu_device *adev, + enum amdgpu_sync_mode mode, + void *owner, struct dma_fence *f) +{ + void *fence_owner = amdgpu_sync_get_owner(f); + + /* Always sync to moves, no matter what */ + if (fence_owner == AMDGPU_FENCE_OWNER_UNDEFINED) + return true; + + /* We only want to trigger KFD eviction fences on +* evict or move jobs. Skip KFD fences otherwise. +*/ + if (fence_owner == AMDGPU_FENCE_OWNER_KFD && + owner != AMDGPU_FENCE_OWNER_UNDEFINED) + return false; + + /* Never sync to VM updates either. */ + if (fence_owner == AMDGPU_FENCE_OWNER_VM && + owner != AMDGPU_FENCE_OWNER_UNDEFINED) + return false; + + /* Ignore fences depending on the sync mode */ + switch (mode) { + case AMDGPU_SYNC_ALWAYS: + return true; + + case AMDGPU_SYNC_NE_OWNER: + if (amdgpu_sync_same_dev(adev, f) && + fence_owner == owner) + return false; + break; + + case AMDGPU_SYNC_EQ_OWNER: + if (amdgpu_sync_same_dev(adev, f) && + fence_owner != owner) + return false; + break; + + case AMDGPU_SYNC_EXPLICIT: + return false; + } + + WARN(debug_evictions && fence_owner == AMDGPU_FENCE_OWNER_KFD, +"Adding eviction fence to sync obj"); + return true; +} + /** * amdgpu_sync_resv - sync to a reservation object * @@ -211,67 +262,34 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync, /* always sync to the exclusive fence */ f = dma_resv_excl_fence(resv); - r = amdgpu_sync_fence(sync, f); + dma_fence_chain_for_each(f, f) { + struct dma_fence_chain *chain = to_dma_fence_chain(f); + + if (amdgpu_sync_test_fence(adev, mode, owner, chain ? + chain->fence : f)) { + r = amdgpu_sync_fence(sync, f); + dma_fence_put(f); + if (r) + return r; + break; + } + } flist = dma_resv_shared_list(resv); - if (!flist || r) - return r; + if (!flist) + return 0; for (i = 0; i < flist->shared_count; ++i) { - void *fence_owner; - f = rcu_dereference_protected(flist->shared[i], dma_resv_held(resv)); - fence_owner = amdgpu_sync_get_owner(f); - - /* Always sync to moves, no matter what */ - if (fence_owner == AMDGPU_FENCE_OWNER_UNDEFINED) { + if (amdgpu_sync_test_fence(adev, mode, owner, f)) { r = amdgpu_sync_fence(sync, f); if (r) - break; - } - - /* We only want to trigger KFD eviction fences on -* evict or move jobs. Skip KFD fences otherwise. -*/ - if (fence_owner == AMDGPU_FENCE_OWNER_KFD && - owner != AMDGPU_FENCE_OWNER_UNDEFINED) - continue; - - /* Never sync to VM updates either. */ - if (fence_owner == AMDGPU_FENCE_OWNER_VM && - owner != AMDGPU_FENCE_OWNER_UNDEFINED) - continue; - - /* Ignore fences depending on the sync mode */ - switch (mode) { - case AMDGPU_SYNC_ALWAYS: - break; - - case AMDGPU_SYNC_NE_OWNER: - if (amdgpu_sync_same_dev(adev, f) && - fence_owner == owner) - continue; - break; - - case AMDGPU_SYNC_EQ_OWNER: -
RE: [PATCH 00/24] DC Patches June 10, 2021
[Public] Hi all, This week this patchset was tested on the following systems: HP Envy 360, with Ryzen 5 4500U, with the following display types: eDP 1080p 60hz, 4k 60hz (via USB-C to DP/HDMI), 1440p 144hz (via USB-C to DP/HDMI), 1680*1050 60hz (via USB-C to DP and then DP to DVI/VGA) AMD Ryzen 9 5900H, with the following display types: eDP 1080p 60hz, 4k 60hz (via USB-C to DP/HDMI), 1440p 144hz (via USB-C to DP/HDMI), 1680*1050 60hz (via USB-C to DP and then DP to DVI/VGA) Sapphire Pulse RX5700XT with the following display types: 4k 60hz (via DP/HDMI), 1440p 144hz (via DP/HDMI), 1680*1050 60hz (via DP to DVI/VGA) Reference AMD RX6800 with the following display types: 4k 60hz (via DP/HDMI and USB-C to DP/HDMI), 1440p 144hz (via USB-C to DP/HDMI and USB-C to DP/HDMI), 1680*1050 60hz (via DP to DVI/VGA) Included testing using a Startech DP 1.4 MST hub at 2x 4k 60hz, and 3x 1080p 60hz on all systems. Tested-by: Daniel Wheeler Thank you, Dan Wheeler Technologist | AMD SW Display -- 1 Commerce Valley Dr E, Thornhill, ON L3T 7X6 Facebook | Twitter | amd.com -Original Message- From: amd-gfx On Behalf Of Anson Jacob Sent: June 10, 2021 12:28 PM To: amd-gfx@lists.freedesktop.org Cc: Brol, Eryk ; Li, Sun peng (Leo) ; Wentland, Harry ; Zhuo, Qingqing ; Siqueira, Rodrigo ; Li, Roman ; Jacob, Anson ; Pillai, Aurabindo ; Lakha, Bhawanpreet ; R, Bindu Subject: [PATCH 00/24] DC Patches June 10, 2021 This DC patchset brings improvements in multiple areas. In summary, we have: * LTTPR improvements * Backlight improvements * eDP hotplug detection *** BLURB HERE *** Anthony Koo (1): drm/amd/display: [FW Promotion] Release 0.0.70 Aric Cyr (1): drm/amd/display: 3.2.140 Ashley Thomas (1): drm/amd/display: add DMUB registers to crash dump diagnostic data. David Galiffi (1): drm/amd/display: Updated variable name. Dmytro Laktyushkin (1): drm/amd/display: Remove unnecessary blank lines Josip Pavic (1): drm/amd/display: tune backlight ramping profiles Po-Ting Chen (1): drm/amd/display: Change swizzle visual confirm reference pipe Roman Li (1): drm/amd/display: move psr dm interface to separate files Wenjing Liu (1): drm/amd/display: dp mst detection code refactor Wesley Chalmers (14): drm/amd/display: Read LTTPR caps first on hotplug drm/amd/display: Move LTTPR cap read into its own function drm/amd/display: Read LTTPR caps first on bootup drm/amd/display: Set LTTPR Transparent Mode after read link cap drm/amd/display: Always write repeater mode regardless of LTTPR drm/amd/display: Improve logic for is_lttpr_present drm/amd/display: Enforce DPCD Address ranges drm/amd/display: Rename constant drm/amd/display: 7 retries + 50 ms timeout on AUX DEFER drm/amd/display: Do not count I2C DEFERs with AUX DEFERs drm/amd/display: Partition DPCD address space and break up transactions drm/amd/display: Add interface to get Calibrated Avg Level from FIFO drm/amd/display: Cover edge-case when changing DISPCLK WDIVIDER drm/amd/display: Extend AUX timeout for DP initial reads Yi-Ling Chen (1): drm/amd/display: add config option for eDP hotplug detection .../gpu/drm/amd/display/amdgpu_dm/Makefile| 2 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 137 +-- .../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c | 166 + .../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.h | 37 +++ drivers/gpu/drm/amd/display/dc/Makefile | 2 +- .../display/dc/clk_mgr/dcn20/dcn20_clk_mgr.c | 68 +- .../display/dc/clk_mgr/dcn20/dcn20_clk_mgr.h | 3 +- .../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c | 4 +- .../drm/amd/display/dc/core/dc_hw_sequencer.c | 10 +- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 167 ++ .../gpu/drm/amd/display/dc/core/dc_link_dp.c | 170 +++--- .../drm/amd/display/dc/core/dc_link_dpcd.c| 218 ++ .../drm/amd/display/dc/core/dc_link_hwss.c| 31 +-- drivers/gpu/drm/amd/display/dc/dc.h | 3 +- drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 100 +++- drivers/gpu/drm/amd/display/dc/dc_dmub_srv.h | 4 + drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 23 +- .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 5 + .../display/dc/dcn10/dcn10_stream_encoder.h | 24 ++ .../display/dc/dcn20/dcn20_stream_encoder.c | 12 + .../display/dc/dcn20/dcn20_stream_encoder.h | 3 + .../dc/dcn30/dcn30_dio_stream_encoder.c | 2 + .../dc/dcn30/dcn30_dio_stream_encoder.h | 12 + .../drm/amd/display/dc/dcn30/dcn30_hwseq.c| 4 + .../drm/amd/display/dc/dcn31/dcn31_hwseq.c| 1 + .../drm/amd/display/dc/dml/display_mode_vba.c | 2 - .../gpu/drm/amd/display/dc/hdcp/hdcp_msg.c| 1 + .../gpu/drm/amd/display/dc/inc/dc_link_dp.h | 11 +- .../amd/display/dc/inc/hw/stream_encoder.h| 3 +
Re: [PATCH 1/5] dma-buf: fix dma_resv_test_signaled test_all handling
Am 11.06.21 um 16:55 schrieb Daniel Vetter: On Fri, Jun 11, 2021 at 04:53:11PM +0200, Christian König wrote: Am 11.06.21 um 16:47 schrieb Daniel Vetter: On Fri, Jun 11, 2021 at 02:02:57PM +0200, Christian König wrote: As the name implies if testing all fences is requested we should indeed test all fences and not skip the exclusive one because we see shared ones. Signed-off-by: Christian König Hm I thought we've had the rule that when both fences exist, then collectively the shared ones must signale no earlier than the exclusive one. That's at least the contract we've implemented in dma_resv.h. But I've also found a bunch of drivers who are a lot more yolo on this. I think there's a solid case here to just always take all the fences if we ask for all the shared ones, but if we go that way then I'd say - clear kerneldoc patch to really hammer this in (currently we're not good at all in this regard) - going through drivers a bit to check for this (I have some of that done already in my earlier series, need to respin it and send it out) But I'm kinda not seeing why this needs to be in this patch series here. You mentioned that this is a problem in the last patch and if you ask me that's just a bug or at least very inconsistent. See dma_resv_wait_timeout() always waits for all fences, including the exclusive one even if shared ones are present. But dma_resv_test_signaled() ignores the exclusive one if shared ones are present. Hm the only one I thought I've mentioned is that dma_buf_poll doesn't use dma_fence_get_rcu_safe where I think it should. Different problem. I think this is one you spotted. The only other driver I could find trying to make use of this is nouveau and I already provided a fix for this as well. i915 also does this, and I think I've found a few more. I just think that this is the more defensive approach to fix this and have at least the core functions consistent on the handling. Oh fully agree, it's just current dma_resv docs aren't the greatest, and hacking on semantics without updating the docs isn't great. Especially when it's ad-hoc. Well when the requirement that shared fences should always signal after the exclusive fence is not documented anywhere then I would say that it is naturally allowed to just add any fence to the list of shared fence and any code assuming something else is just broken and need fixing. Christian. -Daniel Christian. -Daniel --- drivers/dma-buf/dma-resv.c | 33 - 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index f26c71747d43..c66bfdde9454 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -615,25 +615,21 @@ static inline int dma_resv_test_signaled_single(struct dma_fence *passed_fence) */ bool dma_resv_test_signaled(struct dma_resv *obj, bool test_all) { - unsigned int seq, shared_count; + struct dma_fence *fence; + unsigned int seq; int ret; rcu_read_lock(); retry: ret = true; - shared_count = 0; seq = read_seqcount_begin(>seq); if (test_all) { struct dma_resv_list *fobj = dma_resv_shared_list(obj); - unsigned int i; - - if (fobj) - shared_count = fobj->shared_count; + unsigned int i, shared_count; + shared_count = fobj ? fobj->shared_count : 0; for (i = 0; i < shared_count; ++i) { - struct dma_fence *fence; - fence = rcu_dereference(fobj->shared[i]); ret = dma_resv_test_signaled_single(fence); if (ret < 0) @@ -641,24 +637,19 @@ bool dma_resv_test_signaled(struct dma_resv *obj, bool test_all) else if (!ret) break; } - - if (read_seqcount_retry(>seq, seq)) - goto retry; } - if (!shared_count) { - struct dma_fence *fence_excl = dma_resv_excl_fence(obj); - - if (fence_excl) { - ret = dma_resv_test_signaled_single(fence_excl); - if (ret < 0) - goto retry; + fence = dma_resv_excl_fence(obj); + if (fence) { + ret = dma_resv_test_signaled_single(fence); + if (ret < 0) + goto retry; - if (read_seqcount_retry(>seq, seq)) - goto retry; - } } + if (read_seqcount_retry(>seq, seq)) + goto retry; + rcu_read_unlock(); return ret; } -- 2.25.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 1/1] drm/amdgpu: remove amdgpu_vm_pt
Page table entries are now in embedded in VM BO, so we do not need struct amdgpu_vm_pt. This patch replaces struct amdgpu_vm_pt with struct amdgpu_vm_bo_base. v2: change "!(cursor->level < AMDGPU_VM_PTB)" --> "(cursor->level == AMDGPU_VM_PTB)" Signed-off-by: Nirmoy Das --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 26 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 12 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 164 -- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h| 9 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- 12 files changed, 105 insertions(+), 126 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index fb6bcc386de1..f96598279593 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -356,7 +356,7 @@ static int amdgpu_amdkfd_validate_vm_bo(void *_unused, struct amdgpu_bo *bo) */ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) { - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev); int ret; @@ -372,7 +372,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) return ret; } - vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.base.bo); + vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.bo); if (vm->use_cpu_for_update) { ret = amdgpu_bo_kmap(pd, NULL); @@ -387,7 +387,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) static int vm_update_pds(struct amdgpu_vm *vm, struct amdgpu_sync *sync) { - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev); int ret; @@ -1153,7 +1153,7 @@ static int process_sync_pds_resv(struct amdkfd_process_info *process_info, list_for_each_entry(peer_vm, _info->vm_list_head, vm_list_node) { - struct amdgpu_bo *pd = peer_vm->root.base.bo; + struct amdgpu_bo *pd = peer_vm->root.bo; ret = amdgpu_sync_resv(NULL, sync, pd->tbo.base.resv, AMDGPU_SYNC_NE_OWNER, @@ -1220,7 +1220,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, vm->process_info = *process_info; /* Validate page directory and attach eviction fence */ - ret = amdgpu_bo_reserve(vm->root.base.bo, true); + ret = amdgpu_bo_reserve(vm->root.bo, true); if (ret) goto reserve_pd_fail; ret = vm_validate_pt_pd_bos(vm); @@ -1228,16 +1228,16 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, pr_err("validate_pt_pd_bos() failed\n"); goto validate_pd_fail; } - ret = amdgpu_bo_sync_wait(vm->root.base.bo, + ret = amdgpu_bo_sync_wait(vm->root.bo, AMDGPU_FENCE_OWNER_KFD, false); if (ret) goto wait_pd_fail; - ret = dma_resv_reserve_shared(vm->root.base.bo->tbo.base.resv, 1); + ret = dma_resv_reserve_shared(vm->root.bo->tbo.base.resv, 1); if (ret) goto reserve_shared_fail; - amdgpu_bo_fence(vm->root.base.bo, + amdgpu_bo_fence(vm->root.bo, >process_info->eviction_fence->base, true); - amdgpu_bo_unreserve(vm->root.base.bo); + amdgpu_bo_unreserve(vm->root.bo); /* Update process info */ mutex_lock(>process_info->lock); @@ -1251,7 +1251,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, reserve_shared_fail: wait_pd_fail: validate_pd_fail: - amdgpu_bo_unreserve(vm->root.base.bo); + amdgpu_bo_unreserve(vm->root.bo); reserve_pd_fail: vm->process_info = NULL; if (info) { @@ -1306,7 +1306,7 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev, struct amdgpu_vm *vm) { struct amdkfd_process_info *process_info = vm->process_info; - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; if (!process_info) return; @@ -1362,7 +1362,7 @@ void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *drm_priv) uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *drm_priv) { struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv); - struct amdgpu_bo *pd =
Re: [PATCH 1/1] drm/amdgpu: remove amdgpu_vm_pt
Am 10.06.21 um 19:24 schrieb Nirmoy Das: Page table entries are now in embedded in VM BO, so we do not need struct amdgpu_vm_pt. This patch replaces struct amdgpu_vm_pt with struct amdgpu_vm_bo_base. Even struct amdgpu_vm_bo_base should be removed at some point, but for now that's ok. Signed-off-by: Nirmoy Das --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 26 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 12 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 164 -- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h| 9 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- 12 files changed, 105 insertions(+), 126 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index fb6bcc386de1..f96598279593 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -356,7 +356,7 @@ static int amdgpu_amdkfd_validate_vm_bo(void *_unused, struct amdgpu_bo *bo) */ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) { - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev); int ret; @@ -372,7 +372,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) return ret; } - vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.base.bo); + vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.bo); if (vm->use_cpu_for_update) { ret = amdgpu_bo_kmap(pd, NULL); @@ -387,7 +387,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) static int vm_update_pds(struct amdgpu_vm *vm, struct amdgpu_sync *sync) { - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev); int ret; @@ -1153,7 +1153,7 @@ static int process_sync_pds_resv(struct amdkfd_process_info *process_info, list_for_each_entry(peer_vm, _info->vm_list_head, vm_list_node) { - struct amdgpu_bo *pd = peer_vm->root.base.bo; + struct amdgpu_bo *pd = peer_vm->root.bo; ret = amdgpu_sync_resv(NULL, sync, pd->tbo.base.resv, AMDGPU_SYNC_NE_OWNER, @@ -1220,7 +1220,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, vm->process_info = *process_info; /* Validate page directory and attach eviction fence */ - ret = amdgpu_bo_reserve(vm->root.base.bo, true); + ret = amdgpu_bo_reserve(vm->root.bo, true); if (ret) goto reserve_pd_fail; ret = vm_validate_pt_pd_bos(vm); @@ -1228,16 +1228,16 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, pr_err("validate_pt_pd_bos() failed\n"); goto validate_pd_fail; } - ret = amdgpu_bo_sync_wait(vm->root.base.bo, + ret = amdgpu_bo_sync_wait(vm->root.bo, AMDGPU_FENCE_OWNER_KFD, false); if (ret) goto wait_pd_fail; - ret = dma_resv_reserve_shared(vm->root.base.bo->tbo.base.resv, 1); + ret = dma_resv_reserve_shared(vm->root.bo->tbo.base.resv, 1); if (ret) goto reserve_shared_fail; - amdgpu_bo_fence(vm->root.base.bo, + amdgpu_bo_fence(vm->root.bo, >process_info->eviction_fence->base, true); - amdgpu_bo_unreserve(vm->root.base.bo); + amdgpu_bo_unreserve(vm->root.bo); /* Update process info */ mutex_lock(>process_info->lock); @@ -1251,7 +1251,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, reserve_shared_fail: wait_pd_fail: validate_pd_fail: - amdgpu_bo_unreserve(vm->root.base.bo); + amdgpu_bo_unreserve(vm->root.bo); reserve_pd_fail: vm->process_info = NULL; if (info) { @@ -1306,7 +1306,7 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev, struct amdgpu_vm *vm) { struct amdkfd_process_info *process_info = vm->process_info; - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; if (!process_info) return; @@ -1362,7 +1362,7 @@ void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *drm_priv) uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *drm_priv) { struct amdgpu_vm *avm
Re: [PATCH v2 2/7] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property
On Thu, Jun 10, 2021 at 9:55 AM Pekka Paalanen wrote: > > On Tue, 8 Jun 2021 19:43:15 +0200 > Werner Sembach wrote: > > > Add a new general drm property "active bpc" which can be used by graphic > > drivers > > to report the applied bit depth per pixel back to userspace. > > Maybe "bit depth per pixel" -> "bit depth per pixel color component" for slightly more clarity? > > While "max bpc" can be used to change the color depth, there was no way to > > check > > which one actually got used. While in theory the driver chooses the > > best/highest > > color depth within the max bpc setting a user might not be fully aware what > > his > > hardware is or isn't capable off. This is meant as a quick way to double > > check > > the setup. > > > > In the future, automatic color calibration for screens might also depend on > > this > > information being available. > > > > Signed-off-by: Werner Sembach > > --- > > drivers/gpu/drm/drm_atomic_uapi.c | 2 ++ > > drivers/gpu/drm/drm_connector.c | 41 +++ > > include/drm/drm_connector.h | 15 +++ > > 3 files changed, 58 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > b/drivers/gpu/drm/drm_atomic_uapi.c > > index 268bb69c2e2f..7ae4e40936b5 100644 > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > @@ -873,6 +873,8 @@ drm_atomic_connector_get_property(struct drm_connector > > *connector, > > *val = 0; > > } else if (property == connector->max_bpc_property) { > > *val = state->max_requested_bpc; > > + } else if (property == connector->active_bpc_property) { > > + *val = state->active_bpc; > > } else if (connector->funcs->atomic_get_property) { > > return connector->funcs->atomic_get_property(connector, > > state, property, val); > > diff --git a/drivers/gpu/drm/drm_connector.c > > b/drivers/gpu/drm/drm_connector.c > > index 7631f76e7f34..c0c3c09bfed0 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -1195,6 +1195,14 @@ static const struct drm_prop_enum_list > > dp_colorspaces[] = { > > * drm_connector_attach_max_bpc_property() to create and attach the > > * property to the connector during initialization. > > * > > + * active bpc: > > + * This read-only range property tells userspace the pixel color bit > > depth > > + * actually used by the hardware display engine on "the cable" on a > > + * connector. The chosen value depends on hardware capabilities, both > > + * display engine and connected monitor, and the "max bpc" property. > > + * Drivers shall use drm_connector_attach_active_bpc_property() to > > install > > + * this property. > > + * > > This description is now clear to me, but I wonder, is it also how > others understand it wrt. dithering? > > Dithering done on monitor is irrelevant, because we are talking about > "on the cable" pixels. But since we are talking about "on the cable" > pixels, also dithering done by the display engine must not factor in. > Should the dithering done by display engine result in higher "active > bpc" number than what is actually transmitted on the cable? > > I cannot guess what userspace would want exactly. I think the > strict "on the cable" interpretation is a safe bet, because it then > gives a lower limit on observed bpc. Dithering settings should be > exposed with other KMS properties, so userspace can factor those in. > But to be absolutely sure, we'd have to ask some color management > experts. > > Cc'ing Mario in case he has an opinion. > Thanks. I like this a lot, in fact such a connector property was on my todo list / wish list for something like that! I agree with the "active bpc" definition here in this patch and Pekka's comments. I want what goes out over the cable, not including any effects of dithering. At least AMD's amdpu-kms driver exposes "active bpc" already as a per-connector property in debugfs, and i use reported output from there a lot to debug problems with respect to HDR display or high color precision output, and to verify i'm not fooling myself wrt. what goes out, compared to what dithering may "fake" on top of it. Software like mine would greatly benefit from getting this directly off the connector, ie. as a RandR output property, just like with "max bpc", as mapping X11 output names to driver output names is a guessing game, directing regular users to those debugfs files is tedious and error prone, and many regular users don't have root permissions anyway. Sometimes one wants to prioritize "active bpc" over resolution or refresh rate, and especially on now more common HDR displays, and actual bit depth also changes depending on bandwidth requirements vs. availability, and how well DP link training went with a flaky or loose cable, like only getting 10 bpc for HDR-10 when running on less than maximum
Re: [PATCH 1/1] drm/amdgpu: remove amdgpu_vm_pt
ping. On 6/10/2021 7:24 PM, Nirmoy Das wrote: Page table entries are now in embedded in VM BO, so we do not need struct amdgpu_vm_pt. This patch replaces struct amdgpu_vm_pt with struct amdgpu_vm_bo_base. Signed-off-by: Nirmoy Das --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 26 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 12 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 164 -- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h| 9 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- 12 files changed, 105 insertions(+), 126 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index fb6bcc386de1..f96598279593 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -356,7 +356,7 @@ static int amdgpu_amdkfd_validate_vm_bo(void *_unused, struct amdgpu_bo *bo) */ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) { - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev); int ret; @@ -372,7 +372,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) return ret; } - vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.base.bo); + vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.bo); if (vm->use_cpu_for_update) { ret = amdgpu_bo_kmap(pd, NULL); @@ -387,7 +387,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm) static int vm_update_pds(struct amdgpu_vm *vm, struct amdgpu_sync *sync) { - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev); int ret; @@ -1153,7 +1153,7 @@ static int process_sync_pds_resv(struct amdkfd_process_info *process_info, list_for_each_entry(peer_vm, _info->vm_list_head, vm_list_node) { - struct amdgpu_bo *pd = peer_vm->root.base.bo; + struct amdgpu_bo *pd = peer_vm->root.bo; ret = amdgpu_sync_resv(NULL, sync, pd->tbo.base.resv, AMDGPU_SYNC_NE_OWNER, @@ -1220,7 +1220,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, vm->process_info = *process_info; /* Validate page directory and attach eviction fence */ - ret = amdgpu_bo_reserve(vm->root.base.bo, true); + ret = amdgpu_bo_reserve(vm->root.bo, true); if (ret) goto reserve_pd_fail; ret = vm_validate_pt_pd_bos(vm); @@ -1228,16 +1228,16 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, pr_err("validate_pt_pd_bos() failed\n"); goto validate_pd_fail; } - ret = amdgpu_bo_sync_wait(vm->root.base.bo, + ret = amdgpu_bo_sync_wait(vm->root.bo, AMDGPU_FENCE_OWNER_KFD, false); if (ret) goto wait_pd_fail; - ret = dma_resv_reserve_shared(vm->root.base.bo->tbo.base.resv, 1); + ret = dma_resv_reserve_shared(vm->root.bo->tbo.base.resv, 1); if (ret) goto reserve_shared_fail; - amdgpu_bo_fence(vm->root.base.bo, + amdgpu_bo_fence(vm->root.bo, >process_info->eviction_fence->base, true); - amdgpu_bo_unreserve(vm->root.base.bo); + amdgpu_bo_unreserve(vm->root.bo); /* Update process info */ mutex_lock(>process_info->lock); @@ -1251,7 +1251,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info, reserve_shared_fail: wait_pd_fail: validate_pd_fail: - amdgpu_bo_unreserve(vm->root.base.bo); + amdgpu_bo_unreserve(vm->root.bo); reserve_pd_fail: vm->process_info = NULL; if (info) { @@ -1306,7 +1306,7 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev, struct amdgpu_vm *vm) { struct amdkfd_process_info *process_info = vm->process_info; - struct amdgpu_bo *pd = vm->root.base.bo; + struct amdgpu_bo *pd = vm->root.bo; if (!process_info) return; @@ -1362,7 +1362,7 @@ void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *drm_priv) uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *drm_priv) { struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv); - struct amdgpu_bo *pd = avm->root.base.bo; +
Re: [PATCH] drm/amd/display: Verify Gamma & Degamma LUT sizes in amdgpu_dm_atomic_check
hmm I see, thanks for the heads up, I'll double check why it uses google email for sending. wrt the assignment in the if clauses, are those typically frowned upon? Thanks! On Fri, Jun 11, 2021 at 4:17 PM Alex Deucher wrote: > > Just a heads up, your sender email and your signed-off-by don't match > and you had some assignments in if clauses. I've fixed those up. > > Alex > > > On Fri, Jun 11, 2021 at 1:35 PM Alex Deucher wrote: > > > > Applied. Thanks! > > > > On Thu, Jun 10, 2021 at 5:14 PM Harry Wentland > > wrote: > > > > > > > > > > > > On 2021-06-07 10:53 a.m., Mark Yacoub wrote: > > > > On Fri, Jun 4, 2021 at 4:17 PM Harry Wentland > > > > wrote: > > > >> > > > >> > > > >> > > > >> On 2021-06-04 1:01 p.m., Mark Yacoub wrote: > > > >>> From: Mark Yacoub > > > >>> > > > >>> For each CRTC state, check the size of Gamma and Degamma LUTs so > > > >>> unexpected and larger sizes wouldn't slip through. > > > >>> > > > >>> TEST: IGT:kms_color::pipe-invalid-gamma-lut-sizes > > > >>> > > > >>> Signed-off-by: Mark Yacoub > > > >>> Change-Id: I9d513a38e8ac2af1b4bf802e1feb1a4d726fba4c > > > >>> --- > > > >>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++ > > > >>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 + > > > >>> .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 40 > > > >>> --- > > > >>> 3 files changed, 38 insertions(+), 6 deletions(-) > > > >>> > > > >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > >>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > >>> index 38d497d30dba8..f6cd522b42a80 100644 > > > >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > >>> @@ -9402,6 +9402,9 @@ static int amdgpu_dm_atomic_check(struct > > > >>> drm_device *dev, > > > >>> dm_old_crtc_state->dsc_force_changed == false) > > > >>> continue; > > > >>> > > > >>> + if ((ret = amdgpu_dm_verify_lut_sizes(new_crtc_state))) > > > >>> + goto fail; > > > >>> + > > > >>> if (!new_crtc_state->enable) > > > >>> continue; > > > >>> > > > >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > > > >>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > > > >>> index 8bfe901cf2374..1b77cd2612691 100644 > > > >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > > > >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > > > >>> @@ -541,6 +541,7 @@ void amdgpu_dm_trigger_timing_sync(struct > > > >>> drm_device *dev); > > > >>> #define MAX_COLOR_LEGACY_LUT_ENTRIES 256 > > > >>> > > > >>> void amdgpu_dm_init_color_mod(void); > > > >>> +int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state > > > >>> *crtc_state); > > > >>> int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc); > > > >>> int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc, > > > >>> struct dc_plane_state > > > >>> *dc_plane_state); > > > >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c > > > >>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c > > > >>> index 157fe4efbb599..da6f9fcc0b415 100644 > > > >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c > > > >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c > > > >>> @@ -284,6 +284,37 @@ static int __set_input_tf(struct > > > >>> dc_transfer_func *func, > > > >>> return res ? 0 : -ENOMEM; > > > >>> } > > > >>> > > > >>> +/** > > > >>> + * Verifies that the Degamma and Gamma LUTs attached to the > > > >>> |crtc_state| are of > > > >>> + * the expected size. > > > >>> + * Returns 0 on success. > > > >>> + */ > > > >>> +int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state > > > >>> *crtc_state) > > > >>> +{ > > > >>> + const struct drm_color_lut *lut = NULL; > > > >>> + uint32_t size = 0; > > > >>> + > > > >>> + lut = __extract_blob_lut(crtc_state->degamma_lut, ); > > > >>> + if (lut && size != MAX_COLOR_LUT_ENTRIES) { > > > >> > > > >> Isn't the point of the LUT size that it can be variable? Did you > > > >> observe any > > > >> problems with LUTs that are not of size 4096? > > > > Is it supposed to be variable? > > > > I'm basing my knowledge of LUTs on this IGT Test: > > > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_color_helper.c#L281>> > > > > It does check for invalid sizes and for the exact size, giving me the > > > > impression that it's not too flexible. > > > > Is variability of size an AMD specific behavior or should it be a DRM > > > > behavior? > > > >> > > > >> Legacy X-based userspace will give us 256 size LUTs. We can't break > > > >> support for > > > >> that. See MAX_COLOR_LEGACY_LUT_ENTRIES. > > > > In the new function `amdgpu_dm_verify_lut_sizes`, I maintained parity > > > > with the old behavior. In
Re: [PATCH] drm/amd/display: Verify Gamma & Degamma LUT sizes in amdgpu_dm_atomic_check
On Mon, Jun 14, 2021 at 10:49 AM Mark Yacoub wrote: > > hmm I see, thanks for the heads up, I'll double check why it uses > google email for sending. > wrt the assignment in the if clauses, are those typically frowned upon? Yes, checkpatch complains about them. Alex > Thanks! > > On Fri, Jun 11, 2021 at 4:17 PM Alex Deucher wrote: > > > > Just a heads up, your sender email and your signed-off-by don't match > > and you had some assignments in if clauses. I've fixed those up. > > > > Alex > > > > > > On Fri, Jun 11, 2021 at 1:35 PM Alex Deucher wrote: > > > > > > Applied. Thanks! > > > > > > On Thu, Jun 10, 2021 at 5:14 PM Harry Wentland > > > wrote: > > > > > > > > > > > > > > > > On 2021-06-07 10:53 a.m., Mark Yacoub wrote: > > > > > On Fri, Jun 4, 2021 at 4:17 PM Harry Wentland > > > > > wrote: > > > > >> > > > > >> > > > > >> > > > > >> On 2021-06-04 1:01 p.m., Mark Yacoub wrote: > > > > >>> From: Mark Yacoub > > > > >>> > > > > >>> For each CRTC state, check the size of Gamma and Degamma LUTs so > > > > >>> unexpected and larger sizes wouldn't slip through. > > > > >>> > > > > >>> TEST: IGT:kms_color::pipe-invalid-gamma-lut-sizes > > > > >>> > > > > >>> Signed-off-by: Mark Yacoub > > > > >>> Change-Id: I9d513a38e8ac2af1b4bf802e1feb1a4d726fba4c > > > > >>> --- > > > > >>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++ > > > > >>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 + > > > > >>> .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 40 > > > > >>> --- > > > > >>> 3 files changed, 38 insertions(+), 6 deletions(-) > > > > >>> > > > > >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > > >>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > > >>> index 38d497d30dba8..f6cd522b42a80 100644 > > > > >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > > >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > > >>> @@ -9402,6 +9402,9 @@ static int amdgpu_dm_atomic_check(struct > > > > >>> drm_device *dev, > > > > >>> dm_old_crtc_state->dsc_force_changed == false) > > > > >>> continue; > > > > >>> > > > > >>> + if ((ret = > > > > >>> amdgpu_dm_verify_lut_sizes(new_crtc_state))) > > > > >>> + goto fail; > > > > >>> + > > > > >>> if (!new_crtc_state->enable) > > > > >>> continue; > > > > >>> > > > > >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > > > > >>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > > > > >>> index 8bfe901cf2374..1b77cd2612691 100644 > > > > >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > > > > >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > > > > >>> @@ -541,6 +541,7 @@ void amdgpu_dm_trigger_timing_sync(struct > > > > >>> drm_device *dev); > > > > >>> #define MAX_COLOR_LEGACY_LUT_ENTRIES 256 > > > > >>> > > > > >>> void amdgpu_dm_init_color_mod(void); > > > > >>> +int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state > > > > >>> *crtc_state); > > > > >>> int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc); > > > > >>> int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc, > > > > >>> struct dc_plane_state > > > > >>> *dc_plane_state); > > > > >>> diff --git > > > > >>> a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c > > > > >>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c > > > > >>> index 157fe4efbb599..da6f9fcc0b415 100644 > > > > >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c > > > > >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c > > > > >>> @@ -284,6 +284,37 @@ static int __set_input_tf(struct > > > > >>> dc_transfer_func *func, > > > > >>> return res ? 0 : -ENOMEM; > > > > >>> } > > > > >>> > > > > >>> +/** > > > > >>> + * Verifies that the Degamma and Gamma LUTs attached to the > > > > >>> |crtc_state| are of > > > > >>> + * the expected size. > > > > >>> + * Returns 0 on success. > > > > >>> + */ > > > > >>> +int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state > > > > >>> *crtc_state) > > > > >>> +{ > > > > >>> + const struct drm_color_lut *lut = NULL; > > > > >>> + uint32_t size = 0; > > > > >>> + > > > > >>> + lut = __extract_blob_lut(crtc_state->degamma_lut, ); > > > > >>> + if (lut && size != MAX_COLOR_LUT_ENTRIES) { > > > > >> > > > > >> Isn't the point of the LUT size that it can be variable? Did you > > > > >> observe any > > > > >> problems with LUTs that are not of size 4096? > > > > > Is it supposed to be variable? > > > > > I'm basing my knowledge of LUTs on this IGT Test: > > > > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_color_helper.c#L281>> > > > > > It does check for invalid sizes and for the exact size, giving me the > > > > > impression that it's not too flexible. > > > > > Is variability of size
Re: [PATCH V2] treewide: Add missing semicolons to __assign_str uses
On Sat, 12 Jun 2021 08:42:27 -0700 Joe Perches wrote: > The __assign_str macro has an unusual ending semicolon but the vast > majority of uses of the macro already have semicolon termination. > > $ git grep -P '\b__assign_str\b' | wc -l > 551 > $ git grep -P '\b__assign_str\b.*;' | wc -l > 480 > > Add semicolons to the __assign_str() uses without semicolon termination > and all the other uses without semicolon termination via additional defines > that are equivalent to __assign_str() with the eventual goal of removing > the semicolon from the __assign_str() macro definition. > > Link: > https://lore.kernel.org/lkml/1e068d21106bb6db05b735b4916bb420e6c9842a.ca...@perches.com/ FYI, please send new patches as new threads. Otherwise it is likely to be missed. -- Steve ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH V2] treewide: Add missing semicolons to __assign_str uses
The __assign_str macro has an unusual ending semicolon but the vast majority of uses of the macro already have semicolon termination. $ git grep -P '\b__assign_str\b' | wc -l 551 $ git grep -P '\b__assign_str\b.*;' | wc -l 480 Add semicolons to the __assign_str() uses without semicolon termination and all the other uses without semicolon termination via additional defines that are equivalent to __assign_str() with the eventual goal of removing the semicolon from the __assign_str() macro definition. Link: https://lore.kernel.org/lkml/1e068d21106bb6db05b735b4916bb420e6c9842a.ca...@perches.com/ Signed-off-by: Joe Perches --- V2: Remove semicolon addition to #define VIF_ASSIGN as every use of this macro already has a semicolon termination. Compiled x84-64 allyesconfig drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 14 drivers/gpu/drm/lima/lima_trace.h | 2 +- drivers/infiniband/hw/hfi1/trace_misc.h| 4 +-- drivers/infiniband/hw/hfi1/trace_rc.h | 4 +-- drivers/infiniband/hw/hfi1/trace_tid.h | 6 ++-- drivers/infiniband/hw/hfi1/trace_tx.h | 8 ++--- drivers/infiniband/sw/rdmavt/trace_cq.h| 4 +-- drivers/infiniband/sw/rdmavt/trace_mr.h| 2 +- drivers/infiniband/sw/rdmavt/trace_qp.h| 4 +-- drivers/infiniband/sw/rdmavt/trace_rc.h| 2 +- drivers/infiniband/sw/rdmavt/trace_tx.h| 4 +-- drivers/misc/mei/mei-trace.h | 6 ++-- .../net/ethernet/marvell/octeontx2/af/rvu_trace.h | 12 +++ drivers/net/fjes/fjes_trace.h | 4 +-- drivers/usb/cdns3/cdnsp-trace.h| 2 +- fs/nfs/nfs4trace.h | 6 ++-- fs/nfs/nfstrace.h | 4 +-- include/trace/events/btrfs.h | 2 +- include/trace/events/dma_fence.h | 4 +-- include/trace/events/rpcgss.h | 4 +-- include/trace/events/sunrpc.h | 40 +++--- 21 files changed, 69 insertions(+), 69 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h index 0527772fe1b80..d855cb53c7e09 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h @@ -176,10 +176,10 @@ TRACE_EVENT(amdgpu_cs_ioctl, TP_fast_assign( __entry->sched_job_id = job->base.id; - __assign_str(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job)) + __assign_str(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job)); __entry->context = job->base.s_fence->finished.context; __entry->seqno = job->base.s_fence->finished.seqno; - __assign_str(ring, to_amdgpu_ring(job->base.sched)->name) + __assign_str(ring, to_amdgpu_ring(job->base.sched)->name); __entry->num_ibs = job->num_ibs; ), TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u", @@ -201,10 +201,10 @@ TRACE_EVENT(amdgpu_sched_run_job, TP_fast_assign( __entry->sched_job_id = job->base.id; - __assign_str(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job)) + __assign_str(timeline, AMDGPU_JOB_GET_TIMELINE_NAME(job)); __entry->context = job->base.s_fence->finished.context; __entry->seqno = job->base.s_fence->finished.seqno; - __assign_str(ring, to_amdgpu_ring(job->base.sched)->name) + __assign_str(ring, to_amdgpu_ring(job->base.sched)->name); __entry->num_ibs = job->num_ibs; ), TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, ring_name=%s, num_ibs=%u", @@ -229,7 +229,7 @@ TRACE_EVENT(amdgpu_vm_grab_id, TP_fast_assign( __entry->pasid = vm->pasid; - __assign_str(ring, ring->name) + __assign_str(ring, ring->name); __entry->vmid = job->vmid; __entry->vm_hub = ring->funcs->vmhub, __entry->pd_addr = job->vm_pd_addr; @@ -424,7 +424,7 @@ TRACE_EVENT(amdgpu_vm_flush, ), TP_fast_assign( - __assign_str(ring, ring->name) + __assign_str(ring, ring->name); __entry->vmid = vmid; __entry->vm_hub = ring->funcs->vmhub; __entry->pd_addr = pd_addr; @@ -525,7 +525,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync,
[PATCH --next] drm/amd/amdgpu: Fix kernel doc warnings
Commit cb1c81467af3 ("drm/ttm: flip the switch for driver allocated resources v2") changed few ttm_resource pointer names. The corresponding kernel doc comments were not updated, which produces few kernel doc build warnings of the type: ./drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c:193: warning: Excess function parameter 'mem' description in 'amdgpu_gtt_mgr_del' Rename the kernel doc function arguments to match the prototype. Signed-off-by: Dwaipayan Ray --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index ec96e0b26b11..fa88273364b3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -118,7 +118,7 @@ bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *res) * @man: TTM memory type manager * @tbo: TTM BO we need this range for * @place: placement flags and restrictions - * @mem: the resulting mem object + * @res: the resulting TTM memory object * * Dummy, allocate the node but no space for it yet. */ @@ -184,7 +184,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, * amdgpu_gtt_mgr_del - free ranges * * @man: TTM memory type manager - * @mem: TTM memory object + * @res: TTM memory object * * Free the allocated GTT again. */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c index 436ec246a7da..f06813f04e8c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c @@ -361,7 +361,7 @@ static void amdgpu_vram_mgr_virt_start(struct ttm_resource *mem, * @man: TTM memory type manager * @tbo: TTM BO we need this range for * @place: placement flags and restrictions - * @mem: the resulting mem object + * @res: the resulting TTM memory object * * Allocate VRAM for the given BO. */ @@ -482,7 +482,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man, * amdgpu_vram_mgr_del - free ranges * * @man: TTM memory type manager - * @mem: TTM memory object + * @res: TTM memory object * * Free the allocated VRAM again. */ @@ -517,7 +517,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man, * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table * * @adev: amdgpu device pointer - * @mem: TTM memory object + * @res: TTM memory object * @offset: byte offset from the base of VRAM BO * @length: number of bytes to export in sg_table * @dev: the other device -- 2.28.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 6/7] drm/amdgpu: unwrap fence chains in the explicit sync fence
Am 11.06.21 um 17:18 schrieb Daniel Vetter: On Fri, Jun 11, 2021 at 12:09:19PM +0200, Christian König wrote: Am 11.06.21 um 11:07 schrieb Daniel Vetter: On Thu, Jun 10, 2021 at 11:17:59AM +0200, Christian König wrote: Unwrap a the explicit fence if it is a dma_fence_chain and sync to the first fence not matching the owner rules. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 118 +-- 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c index 1b2ceccaf5b0..862eb3c1c4c5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c @@ -28,6 +28,8 @@ *Christian König */ +#include + #include "amdgpu.h" #include "amdgpu_trace.h" #include "amdgpu_amdkfd.h" @@ -186,6 +188,55 @@ int amdgpu_sync_vm_fence(struct amdgpu_sync *sync, struct dma_fence *fence) return amdgpu_sync_fence(sync, fence); } +/* Determine based on the owner and mode if we should sync to a fence or not */ +static bool amdgpu_sync_test_fence(struct amdgpu_device *adev, + enum amdgpu_sync_mode mode, + void *owner, struct dma_fence *f) +{ + void *fence_owner = amdgpu_sync_get_owner(f); + + /* Always sync to moves, no matter what */ + if (fence_owner == AMDGPU_FENCE_OWNER_UNDEFINED) + return true; + + /* We only want to trigger KFD eviction fences on +* evict or move jobs. Skip KFD fences otherwise. +*/ + if (fence_owner == AMDGPU_FENCE_OWNER_KFD && + owner != AMDGPU_FENCE_OWNER_UNDEFINED) + return false; + + /* Never sync to VM updates either. */ + if (fence_owner == AMDGPU_FENCE_OWNER_VM && + owner != AMDGPU_FENCE_OWNER_UNDEFINED) + return false; + + /* Ignore fences depending on the sync mode */ + switch (mode) { + case AMDGPU_SYNC_ALWAYS: + return true; + + case AMDGPU_SYNC_NE_OWNER: + if (amdgpu_sync_same_dev(adev, f) && + fence_owner == owner) + return false; + break; + + case AMDGPU_SYNC_EQ_OWNER: + if (amdgpu_sync_same_dev(adev, f) && + fence_owner != owner) + return false; + break; + + case AMDGPU_SYNC_EXPLICIT: + return false; + } + + WARN(debug_evictions && fence_owner == AMDGPU_FENCE_OWNER_KFD, +"Adding eviction fence to sync obj"); + return true; +} + /** * amdgpu_sync_resv - sync to a reservation object * @@ -211,67 +262,34 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync, /* always sync to the exclusive fence */ f = dma_resv_excl_fence(resv); - r = amdgpu_sync_fence(sync, f); + dma_fence_chain_for_each(f, f) { Jason has some helper for deep-walking fence chains/arrays here I think. Might want to look into that, so that we have some consistency in how we pile up multiple exclusive fences. Well those helpers are not from Jason, but from me :) But no, for now the deep inspection is not really helpful here since grabbing a reference to a certain chain node is what that makes the handling easier and faster here. Thinking more about it that should also make it possible for the garbage collection to kick in properly. Hm this is tricky to reason about, but yeah with this here it's a true chain, and you just need to connect them. But then if a buffer is on multiple engines, collapsing things down occasionally might be useful. But maybe we need to do that in the bigger rework where exclusive fences are also just in the dma_fence_list with a "this is an exclusive one btw" tag. I think for the vk import case doing the deep scan makes more sense, it's a once-per-frame thing, and there's a much bigger chance that you have a pile of fences from different engines on it already. The problem with Jasons IOCTL is that you *must* do a deep dive and flatten out the fences. Otherwise somebody could use it to create a deep fence structure with dma_fence_arrays containing dma_fence_arrays, containing dma_fence_arrays etc... When you then release that structure you overwrite kernel stack because the dma_fence_array does a dma_fence_put() on it's entries :) The dma_fence_chain container is intentionally made in a way to prevent that. I think a comment explaining why we think deep scan isn't a good idea here would be good, just so we can appreciate our foolishness when it all goes wrong :-) Ok, good point. Thanks, Christian. -Daniel Anyway pretty much one of the versions I had in mind too, except I didn't type it up. Acked-by: Daniel Vetter Thanks, Christian. + struct dma_fence_chain *chain =