[PATCH] drm/amdkfd: reserve the BO before validating it

[Lang Yu]

Fixes: 410f08516e0f ("drm/amdkfd: Move dma unmapping after TLB flush")

[   41.708711] WARNING: CPU: 0 PID: 1463 at drivers/gpu/drm/ttm/ttm_bo.c:846 
ttm_bo_validate+0x146/0x1b0 [ttm]
[   41.708989] Call Trace:
[   41.708992]  
[   41.708996]  ? show_regs+0x6c/0x80
[   41.709000]  ? ttm_bo_validate+0x146/0x1b0 [ttm]
[   41.709008]  ? __warn+0x93/0x190
[   41.709014]  ? ttm_bo_validate+0x146/0x1b0 [ttm]
[   41.709024]  ? report_bug+0x1f9/0x210
[   41.709035]  ? handle_bug+0x46/0x80
[   41.709041]  ? exc_invalid_op+0x1d/0x80
[   41.709048]  ? asm_exc_invalid_op+0x1f/0x30
[   41.709057]  ? amdgpu_amdkfd_gpuvm_dmaunmap_mem+0x2c/0x80 [amdgpu]
[   41.709185]  ? ttm_bo_validate+0x146/0x1b0 [ttm]
[   41.709197]  ? amdgpu_amdkfd_gpuvm_dmaunmap_mem+0x2c/0x80 [amdgpu]
[   41.709337]  ? srso_alias_return_thunk+0x5/0x7f
[   41.709346]  kfd_mem_dmaunmap_attachment+0x9e/0x1e0 [amdgpu]
[   41.709467]  amdgpu_amdkfd_gpuvm_dmaunmap_mem+0x56/0x80 [amdgpu]
[   41.709586]  kfd_ioctl_unmap_memory_from_gpu+0x1b7/0x300 [amdgpu]
[   41.709710]  kfd_ioctl+0x1ec/0x650 [amdgpu]
[   41.709822]  ? __pfx_kfd_ioctl_unmap_memory_from_gpu+0x10/0x10 [amdgpu]
[   41.709945]  ? srso_alias_return_thunk+0x5/0x7f
[   41.709949]  ? tomoyo_file_ioctl+0x20/0x30
[   41.709959]  __x64_sys_ioctl+0x9c/0xd0
[   41.709967]  do_syscall_64+0x3f/0x90
[   41.709973]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8

Signed-off-by: Lang Yu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 48697b789342..f5542a4ab8ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -2095,8 +2095,13 @@ void amdgpu_amdkfd_gpuvm_dmaunmap_mem(struct kgd_mem 
*mem, void *drm_priv)
mutex_lock(>lock);
 
list_for_each_entry(entry, >attachments, list) {
-   if (entry->bo_va->base.vm == vm)
+   if (entry->bo_va->base.vm != vm)
+   continue;
+
+   if (!WARN_ON(amdgpu_bo_reserve(entry->bo_va->base.bo, true))) {
kfd_mem_dmaunmap_attachment(mem, entry);
+   amdgpu_bo_unreserve(entry->bo_va->base.bo);
+   }
}
 
mutex_unlock(>lock);
-- 
2.25.1

Re: [PATCH] drm/amdkfd: Fix the shift-out-of-bounds warning

[Ma, Jun]

Hi Felix,

On 1/10/2024 11:57 PM, Felix Kuehling wrote:
> On 2024-01-10 04:39, Ma Jun wrote:
>> There is following shift-out-of-bounds warning if ecode=0.
>> "shift exponent 4294967295 is too large for 64-bit type 'long long unsigned 
>> int'"
>>
>> Signed-off-by: Ma Jun 
>> ---
>>   include/uapi/linux/kfd_ioctl.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
>> index 2aa88afe305b..129325b02a91 100644
>> --- a/include/uapi/linux/kfd_ioctl.h
>> +++ b/include/uapi/linux/kfd_ioctl.h
>> @@ -1004,7 +1004,7 @@ enum kfd_dbg_trap_exception_code {
>>   };
>>   
>>   /* Mask generated by ecode in kfd_dbg_trap_exception_code */
>> -#define KFD_EC_MASK(ecode)  (1ULL << (ecode - 1))
>> +#define KFD_EC_MASK(ecode)  (BIT(ecode) - 1)
> 
> This is not the same thing. We want a bit mask with one bit set. And 
> ecode=1 should set bit 0. ecode=0 is not a valid code and doesn't have a 
> valid mask. You could use BIT((ecode) - 1), but I think that would give 
> you the same warning for ecode=0. I also don't see BIT defined anywhere 
> under include/uapi, so I think using this in the API header would break 
> the build in user mode.
> 
> Where are you seeing the warning about the bad shift exponent? Looks 
> like someone is using the KFD_EC_MASK macro incorrectly. Or if there is 
> a legitimate use of it with ecode=0, then the correct fix would be
> 
This warning is caused by following code in function event_interrupt_wq_v10()

else if (source_id == SOC15_INTSRC_CP_BAD_OPCODE) {
kfd_set_dbg_ev_from_interrupt(dev, pasid,
KFD_DEBUG_DOORBELL_ID(context_id0),
KFD_EC_MASK(KFD_DEBUG_CP_BAD_OP_ECODE(context_id0)),
NULL,
0);
}


> #define KFD_EC_MASK(ecode)((ecode) ? 1ULL << (ecode - 1) : 0ULL)

This can fix the warning.

Regards
Ma Jun
> 
> Regards,
>    Felix
> 
> 
>>   
>>   /* Masks for exception code type checks below */
>>   #define KFD_EC_MASK_QUEUE  (KFD_EC_MASK(EC_QUEUE_WAVE_ABORT) | \

RE: [PATCH] drm/amd/pm: Add error log for smu v13.0.6 reset

[Kamal, Asad]

[AMD Official Use Only - General]

Reviewed-by: Asad Kamal 

Thanks & Regards
Asad

-Original Message-
From: Lazar, Lijo 
Sent: Thursday, January 11, 2024 9:52 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Deucher, Alexander 
; Kamal, Asad 
Subject: [PATCH] drm/amd/pm: Add error log for smu v13.0.6 reset

For all mode-2 reset fail cases, add error log.

Signed-off-by: Lijo Lazar 
---
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 4ebc6b421c2c..7513d1cfeebd 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -2235,17 +2235,18 @@ static int smu_v13_0_6_mode2_reset(struct smu_context 
*smu)
continue;
}

-   if (ret) {
-   dev_err(adev->dev,
-   "failed to send mode2 message \tparam: 0x%08x 
error code %d\n",
-   SMU_RESET_MODE_2, ret);
+   if (ret)
goto out;
-   }
+
} while (ret == -ETIME && timeout);

 out:
mutex_unlock(>message_lock);

+   if (ret)
+   dev_err(adev->dev, "failed to send mode2 reset, error code %d",
+   ret);
+
return ret;
 }

--
2.25.1

[PATCH] drm/amd/pm: Add error log for smu v13.0.6 reset

[Lijo Lazar]

For all mode-2 reset fail cases, add error log.

Signed-off-by: Lijo Lazar 
---
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 4ebc6b421c2c..7513d1cfeebd 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -2235,17 +2235,18 @@ static int smu_v13_0_6_mode2_reset(struct smu_context 
*smu)
continue;
}
 
-   if (ret) {
-   dev_err(adev->dev,
-   "failed to send mode2 message \tparam: 0x%08x 
error code %d\n",
-   SMU_RESET_MODE_2, ret);
+   if (ret)
goto out;
-   }
+
} while (ret == -ETIME && timeout);
 
 out:
mutex_unlock(>message_lock);
 
+   if (ret)
+   dev_err(adev->dev, "failed to send mode2 reset, error code %d",
+   ret);
+
return ret;
 }
 
-- 
2.25.1

Re: [PATCH] drm/amdgpu: revert "Adjust removal control flow for smu v13_0_2"

[Deucher, Alexander]

[Public]

Acked-by: Alex Deucher 

From: Christian König 
Sent: Wednesday, January 10, 2024 9:31 AM
To: Lazar, Lijo ; Chai, Thomas ; 
Deucher, Alexander ; amd-gfx@lists.freedesktop.org 

Subject: [PATCH] drm/amdgpu: revert "Adjust removal control flow for smu 
v13_0_2"

Calling amdgpu_device_ip_resume_phase1() during shutdown leaves the
HW in an active state and is an unbalanced use of the IP callbacks.

Using the IP callbacks like this can lead to memory leaks, double
free and imbalanced reference counters.

Leaving the HW in an active state can lead to DMA accesses to memory now
freed by the driver.

Both is a complete no-go for driver unload so completely revert the
workaround for now.

This reverts commit f5c7e7797060255dbc8160734ccc5ad6183c5e04.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 32 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 32 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  1 -
 4 files changed, 1 insertion(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a39c9fea55c4..313316009039 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5232,7 +5232,6 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 struct amdgpu_device *tmp_adev = NULL;
 bool need_full_reset, skip_hw_reset, vram_lost = false;
 int r = 0;
-   bool gpu_reset_for_dev_remove = 0;

 /* Try reset handler method first */
 tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
@@ -5252,10 +5251,6 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 test_bit(AMDGPU_NEED_FULL_RESET, _context->flags);
 skip_hw_reset = test_bit(AMDGPU_SKIP_HW_RESET, _context->flags);

-   gpu_reset_for_dev_remove =
-   test_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, _context->flags) 
&&
-   test_bit(AMDGPU_NEED_FULL_RESET, _context->flags);
-
 /*
  * ASIC reset has to be done on all XGMI hive nodes ASAP
  * to allow proper links negotiation in FW (within 1 sec)
@@ -5298,18 +5293,6 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 amdgpu_ras_intr_cleared();
 }

-   /* Since the mode1 reset affects base ip blocks, the
-* phase1 ip blocks need to be resumed. Otherwise there
-* will be a BIOS signature error and the psp bootloader
-* can't load kdb on the next amdgpu install.
-*/
-   if (gpu_reset_for_dev_remove) {
-   list_for_each_entry(tmp_adev, device_list_handle, reset_list)
-   amdgpu_device_ip_resume_phase1(tmp_adev);
-
-   goto end;
-   }
-
 list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
 if (need_full_reset) {
 /* post card */
@@ -5543,11 +5526,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 int i, r = 0;
 bool need_emergency_restart = false;
 bool audio_suspended = false;
-   bool gpu_reset_for_dev_remove = false;
-
-   gpu_reset_for_dev_remove =
-   test_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, 
_context->flags) &&
-   test_bit(AMDGPU_NEED_FULL_RESET, 
_context->flags);

 /*
  * Special case: RAS triggered and full reset isn't supported
@@ -5585,7 +5563,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 if (!amdgpu_sriov_vf(adev) && (adev->gmc.xgmi.num_physical_nodes > 1)) 
{
 list_for_each_entry(tmp_adev, >device_list, 
gmc.xgmi.head) {
 list_add_tail(_adev->reset_list, _list);
-   if (gpu_reset_for_dev_remove && adev->shutdown)
+   if (adev->shutdown)
 tmp_adev->shutdown = true;
 }
 if (!list_is_first(>reset_list, _list))
@@ -5670,10 +5648,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,

 retry:  /* Rest of adevs pre asic reset from XGMI hive. */
 list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
-   if (gpu_reset_for_dev_remove) {
-   /* Workaroud for ASICs need to disable SMC first */
-   amdgpu_device_smu_fini_early(tmp_adev);
-   }
 r = amdgpu_device_pre_asic_reset(tmp_adev, reset_context);
 /*TODO Should we stop ?*/
 if (r) {
@@ -5705,9 +5679,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 r = amdgpu_do_asic_reset(device_list_handle, reset_context);
 if (r && r == -EAGAIN)
 goto retry;
-
-   

[PATCH v3] amd/amdkfd: Set correct svm range actual loc after spliting

[Philip Yang]

While svm range partial migrating to system memory, clear dma_addr vram
domain flag, otherwise the future split will get incorrect vram_pages
and actual loc.

After range spliting, set new range and old range actual_loc:
new range actual_loc is 0 if new->vram_pages is 0.
old range actual_loc is 0 if old->vram_pages - new->vram_pages == 0.

new range takes svm_bo ref only if vram_pages not equal to 0.

Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 20 +++-
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 24 ++--
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index f856901055d3..dae05f70257b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -563,18 +563,30 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, 
struct svm_range *prange,
struct migrate_vma *migrate, struct dma_fence **mfence,
dma_addr_t *scratch, uint64_t npages)
 {
+   struct kfd_process *p = container_of(prange->svms, struct kfd_process, 
svms);
struct device *dev = adev->dev;
+   dma_addr_t *dma_addr;
uint64_t *src;
dma_addr_t *dst;
struct page *dpage;
uint64_t i = 0, j;
uint64_t addr;
+   s32 gpuidx;
+   u64 offset;
int r = 0;
 
pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start,
 prange->last);
 
-   addr = prange->start << PAGE_SHIFT;
+   gpuidx = kfd_process_gpuidx_from_gpuid(p, prange->actual_loc);
+   if (gpuidx < 0) {
+   pr_debug("no GPU id 0x%x found\n", prange->actual_loc);
+   return -EINVAL;
+   }
+
+   addr = migrate->start;
+   offset = (addr >> PAGE_SHIFT) - prange->start;
+   dma_addr = prange->dma_addr[gpuidx];
 
src = (uint64_t *)(scratch + npages);
dst = scratch;
@@ -623,6 +635,12 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct 
svm_range *prange,
goto out_oom;
}
 
+   /* Clear VRAM flag when page is migrated to ram, to count vram
+* pages correctly when spliting the range.
+*/
+   if (dma_addr && (dma_addr[offset + i] & SVM_RANGE_VRAM_DOMAIN))
+   dma_addr[offset + i] = 0;
+
pr_debug_ratelimited("dma mapping dst to 0x%llx, pfn 0x%lx\n",
 dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index cc24f30f88fb..35ee9e648cca 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -362,7 +362,6 @@ svm_range *svm_range_new(struct svm_range_list *svms, 
uint64_t start,
INIT_LIST_HEAD(>child_list);
atomic_set(>invalid, 0);
prange->validate_timestamp = 0;
-   prange->vram_pages = 0;
mutex_init(>migrate_mutex);
mutex_init(>lock);
 
@@ -980,9 +979,14 @@ svm_range_split_pages(struct svm_range *new, struct 
svm_range *old,
if (r)
return r;
}
-   if (old->actual_loc)
+   if (old->actual_loc && new->vram_pages) {
old->vram_pages -= new->vram_pages;
-
+   new->actual_loc = old->actual_loc;
+   if (!old->vram_pages)
+   old->actual_loc = 0;
+   }
+   pr_debug("new->vram_pages 0x%llx loc 0x%x old->vram_pages 0x%llx loc 
0x%x\n",
+new->vram_pages, new->actual_loc, old->vram_pages, 
old->actual_loc);
return 0;
 }
 
@@ -1002,13 +1006,14 @@ svm_range_split_nodes(struct svm_range *new, struct 
svm_range *old,
new->offset = old->offset + npages;
}
 
-   new->svm_bo = svm_range_bo_ref(old->svm_bo);
-   new->ttm_res = old->ttm_res;
-
-   spin_lock(>svm_bo->list_lock);
-   list_add(>svm_bo_list, >svm_bo->range_list);
-   spin_unlock(>svm_bo->list_lock);
+   if (new->vram_pages) {
+   new->svm_bo = svm_range_bo_ref(old->svm_bo);
+   new->ttm_res = old->ttm_res;
 
+   spin_lock(>svm_bo->list_lock);
+   list_add(>svm_bo_list, >svm_bo->range_list);
+   spin_unlock(>svm_bo->list_lock);
+   }
return 0;
 }
 
@@ -1058,7 +1063,6 @@ svm_range_split_adjust(struct svm_range *new, struct 
svm_range *old,
new->flags = old->flags;
new->preferred_loc = old->preferred_loc;
new->prefetch_loc = old->prefetch_loc;
-   new->actual_loc = old->actual_loc;
new->granularity = old->granularity;
new->mapped_to_gpu = old->mapped_to_gpu;
bitmap_copy(new->bitmap_access, old->bitmap_access, MAX_GPU_INSTANCE);
-- 
2.35.1

[PATCH 19/19] drm/amd/display: 3.2.267

[Alex Hung]

From: Martin Leung 

- Align the returned error code with legacy DP
- Allow Z8 for multiplane configurations on DCN35
- Set default Z8 minimum residency for DCN35
- Rework DC Z10 restore
- Enable Panel Replay for static screen use case
- Add DP audio BW validation
- Fix dml2 assigned pipe search
- Ensure populate uclk in bb construction
- Update P010 scaling cap
- Reenable windowed mpo odm support
- Fix DML2 watermark calculation
- Clear OPTC mem select on disable
- Floor to mhz when requesting dpp disp clock changes to SMU
- Port DENTIST hang and TDR fixes to OTG disable W/A
- Add logging resource checks
- Add Replay IPS register for DMUB command table
- Init link enc resources in dc_state only if res_pool presents
- Allow IPS2 during Replay

Acked-by: Alex Hung 
Signed-off-by: Martin Leung 
---
 drivers/gpu/drm/amd/display/dc/dc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index c9317ea0258e..1d052742d4c7 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -51,7 +51,7 @@ struct aux_payload;
 struct set_config_cmd_payload;
 struct dmub_notification;
 
-#define DC_VER "3.2.266"
+#define DC_VER "3.2.267"
 
 #define MAX_SURFACES 3
 #define MAX_PLANES 6
-- 
2.34.1

[PATCH 18/19] drm/amd/display: Align the returned error code with legacy DP

[Alex Hung]

From: Wayne Lin 

[Why]
For usb4 connector, AUX transaction is handled by dmub utilizing a differnt
code path comparing to legacy DP connector. If the usb4 DP connector is
disconnected, AUX access will report EBUSY and cause igt@kms_dp_aux_dev
fail.

[How]
Align the error code with the one reported by legacy DP as EIO.

Cc: Mario Limonciello 
Cc: Alex Deucher 
Cc: sta...@vger.kernel.org
Acked-by: Alex Hung 
Signed-off-by: Wayne Lin 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index d3966ce3dc91..e3915c4f8566 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -978,6 +978,11 @@ int dm_helper_dmub_aux_transfer_sync(
struct aux_payload *payload,
enum aux_return_code_type *operation_result)
 {
+   if (!link->hpd_status) {
+   *operation_result = AUX_RET_ERROR_HPD_DISCON;
+   return -1;
+   }
+
return amdgpu_dm_process_dmub_aux_transfer_sync(ctx, link->link_index, 
payload,
operation_result);
 }
-- 
2.34.1

[PATCH 17/19] drm/amd/display: Allow Z8 for multiplane configurations on DCN35

[Alex Hung]

From: Nicholas Kazlauskas 

[Why]
Power improvement over DCN314, but also addresses a functional issue
where plane_state remains uncleared on pipes that aren't actually
active.

[How]
Update the check to allow for zero streams to be treated as z8 allow.
Update the check to remove plane count on the active stream case.

Z8 will still be blocked based on stutter duration, which is likely to
be the case for most multi plane configurations.

Reviewed-by: Gabe Teeger 
Reviewed-by: Charlene Liu 
Acked-by: Alex Hung 
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c
index 475c4ec43c01..a85693caebd5 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c
@@ -583,9 +583,9 @@ void dcn35_decide_zstate_support(struct dc *dc, struct 
dc_state *context)
plane_count++;
}
 
-   if (plane_count == 0) {
+   if (context->stream_count == 0 || plane_count == 0) {
support = DCN_ZSTATE_SUPPORT_ALLOW;
-   } else if (plane_count == 1 && context->stream_count == 1 && 
context->streams[0]->signal == SIGNAL_TYPE_EDP) {
+   } else if (context->stream_count == 1 && context->streams[0]->signal == 
SIGNAL_TYPE_EDP) {
struct dc_link *link = context->streams[0]->sink->link;
bool is_pwrseq0 = link && link->link_index == 0;
bool is_psr1 = link && link->psr_settings.psr_version == 
DC_PSR_VERSION_1 && !link->panel_config.psr.disable_psr;
-- 
2.34.1

[PATCH 16/19] drm/amd/display: Set default Z8 minimum residency for DCN35

[Alex Hung]

From: Nicholas Kazlauskas 

[Why & How]
Match DCN314's policy.

Reviewed-by: Gabe Teeger 
Reviewed-by: Charlene Liu 
Acked-by: Alex Hung 
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c 
b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
index 7d8e957d6a19..5f7cf01abef9 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
@@ -764,6 +764,7 @@ static const struct dc_debug_options debug_defaults_drv = {
},
.seamless_boot_odm_combine = DML_FAIL_SOURCE_PIXEL_FORMAT,
.enable_z9_disable_interface = true, /* Allow support for the PMFW 
interface for disable Z9*/
+   .minimum_z8_residency_time = 2100,
.using_dml2 = true,
.support_eDP1_5 = true,
.enable_hpo_pg_support = false,
-- 
2.34.1

[PATCH 15/19] drm/amd/display: Rework DC Z10 restore

[Alex Hung]

From: Nicholas Kazlauskas 

[Why]
The call currently does two things:
1. Exits DMCUB from idle optimization if it was in
2. Checks DMCUB scratch register to determine if we need to call
   DMCUB to do deferred HW restore and then sends the command if it's
   ready for it.

By doing (1) we prevent driver idle from being renotified in the cases
where driver had previously allowed DC level idle optimizations via
dc_allow_idle_optimizations since it thinks:

allow == dc->idle_optimizations_allowed

...and that the operation is a no-op.

We want driver idle to be resent at the next opprotunity to do so
for video playback cases.

[How]
Migrate all usecases of dc_z10_restore to only perform (2).

Add extra calls to dc_allow_idle_optimizations to handle (1) and also
keep SW state matching with when we requested enter/exit of DMCUB
idle optimizations.

Ensure cursor idle optimizations false always get called when IPS
is supported.

Further rework/redesign is needed to decide whether we need a separate
level of DM allow vs DC allow and when to attempt re-entry.

Reviewed-by: Yihan Zhu 
Reviewed-by: Charlene Liu 
Acked-by: Alex Hung 
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c  | 11 ++-
 drivers/gpu/drm/amd/display/dc/core/dc_stream.c   |  9 +
 .../gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c   |  2 --
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index aa7c02ba948e..af83ec23f3a0 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1836,8 +1836,8 @@ static enum dc_status dc_commit_state_no_check(struct dc 
*dc, struct dc_state *c
struct dc_state *old_state;
bool subvp_prev_use = false;
 
-   dc_z10_restore(dc);
dc_allow_idle_optimizations(dc, false);
+   dc_z10_restore(dc);
 
for (i = 0; i < dc->res_pool->pipe_count; i++) {
struct pipe_ctx *old_pipe = 
>current_state->res_ctx.pipe_ctx[i];
@@ -3376,6 +3376,9 @@ static void commit_planes_for_stream_fast(struct dc *dc,
int i, j;
struct pipe_ctx *top_pipe_to_program = NULL;
struct dc_stream_status *stream_status = NULL;
+   if (dc->caps.ips_support)
+   dc_allow_idle_optimizations(dc, false);
+
dc_z10_restore(dc);
 
top_pipe_to_program = resource_get_otg_master_for_stream(
@@ -3503,6 +3506,9 @@ static void commit_planes_for_stream(struct dc *dc,
// dc->current_state anymore, so we have to cache it before we apply
// the new SubVP context
subvp_prev_use = false;
+   if (dc->caps.ips_support)
+   dc_allow_idle_optimizations(dc, false);
+
dc_z10_restore(dc);
if (update_type == UPDATE_TYPE_FULL)
wait_for_outstanding_hw_updates(dc, context);
@@ -4686,6 +4692,9 @@ void dc_set_power_state(
case DC_ACPI_CM_POWER_STATE_D0:
dc_state_construct(dc, dc->current_state);
 
+   if (dc->caps.ips_support)
+   dc_allow_idle_optimizations(dc, false);
+
dc_z10_restore(dc);
 
dc->hwss.init_hw(dc);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index 54670e0b1518..23f4f3c070cb 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -309,7 +309,6 @@ bool dc_stream_set_cursor_attributes(
 
stream->cursor_attributes = *attributes;
 
-   dc_z10_restore(dc);
/* disable idle optimizations while updating cursor */
if (dc->idle_optimizations_allowed) {
dc_allow_idle_optimizations(dc, false);
@@ -381,12 +380,14 @@ bool dc_stream_set_cursor_position(
}
 
dc = stream->ctx->dc;
-   dc_z10_restore(dc);
 
/* disable idle optimizations if enabling cursor */
-   if (dc->idle_optimizations_allowed && (!stream->cursor_position.enable 
|| dc->debug.exit_idle_opt_for_cursor_updates)
-   && position->enable) {
+   if (dc->idle_optimizations_allowed &&
+   (!stream->cursor_position.enable || 
dc->debug.exit_idle_opt_for_cursor_updates ||
+dc->caps.ips_support) &&
+   position->enable) {
dc_allow_idle_optimizations(dc, false);
+   dc_z10_restore(dc);
reset_idle_optimizations = true;
}
 
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
index 8b6c49622f3b..da4f98de9b82 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
@@ -708,8 +708,6 @@ void dcn35_z10_restore(const struct dc *dc)
if (dc->debug.disable_z10)
return;
 
-   

[PATCH 14/19] drm/amd/display: Enable Panel Replay for static screen use case

[Alex Hung]

From: Tom Chung 

[Why]
Enable the Panel Replay if eDP panel and ASIC support.
(prioritize Panel Replay over PSR)

[How]
- Setup the Panel Replay config during the device init
  (prioritize Panel Replay over PSR).
- Separate the Replay init function into two functions
  amdgpu_dm_link_setup_replay() and amdgpu_dm_set_replay_caps()
  to fix the issue in the earlier commit that cause PSR and Replay
  enabled at the same time.

Reviewed-by: Sun peng Li 
Acked-by: Alex Hung 
Signed-off-by: Tom Chung 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  42 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c|  59 +++--
 .../amd/display/amdgpu_dm/amdgpu_dm_replay.c  | 119 ++
 .../amd/display/amdgpu_dm/amdgpu_dm_replay.h  |   4 +-
 4 files changed, 156 insertions(+), 68 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 95ff3800fc87..f9e41006ac87 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -67,6 +67,7 @@
 #include "amdgpu_dm_debugfs.h"
 #endif
 #include "amdgpu_dm_psr.h"
+#include "amdgpu_dm_replay.h"
 
 #include "ivsrcid/ivsrcid_vislands30.h"
 
@@ -4393,6 +4394,7 @@ static int amdgpu_dm_initialize_drm_device(struct 
amdgpu_device *adev)
enum dc_connection_type new_connection_type = dc_connection_none;
const struct dc_plane_cap *plane;
bool psr_feature_enabled = false;
+   bool replay_feature_enabled = false;
int max_overlay = dm->dc->caps.max_slave_planes;
 
dm->display_indexes_num = dm->dc->caps.max_streams;
@@ -4504,6 +4506,23 @@ static int amdgpu_dm_initialize_drm_device(struct 
amdgpu_device *adev)
}
}
 
+   /* Determine whether to enable Replay support by default. */
+   if (!(amdgpu_dc_debug_mask & DC_DISABLE_REPLAY)) {
+   switch (amdgpu_ip_version(adev, DCE_HWIP, 0)) {
+   case IP_VERSION(3, 1, 4):
+   case IP_VERSION(3, 1, 5):
+   case IP_VERSION(3, 1, 6):
+   case IP_VERSION(3, 2, 0):
+   case IP_VERSION(3, 2, 1):
+   case IP_VERSION(3, 5, 0):
+   replay_feature_enabled = true;
+   break;
+   default:
+   replay_feature_enabled = amdgpu_dc_feature_mask & 
DC_REPLAY_MASK;
+   break;
+   }
+   }
+
/* loops over all connectors on the board */
for (i = 0; i < link_cnt; i++) {
struct dc_link *link = NULL;
@@ -4572,6 +4591,11 @@ static int amdgpu_dm_initialize_drm_device(struct 
amdgpu_device *adev)

amdgpu_dm_update_connector_after_detect(aconnector);
setup_backlight_device(dm, aconnector);
 
+   /* Disable PSR if Replay can be enabled */
+   if (replay_feature_enabled)
+   if (amdgpu_dm_set_replay_caps(link, 
aconnector))
+   psr_feature_enabled = false;
+
if (psr_feature_enabled)
amdgpu_dm_set_psr_caps(link);
 
@@ -8521,10 +8545,17 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
dm_update_pflip_irq_state(drm_to_adev(dev),
  acrtc_attach);
 
-   if ((acrtc_state->update_type > UPDATE_TYPE_FAST) &&
-   
acrtc_state->stream->link->psr_settings.psr_version != 
DC_PSR_VERSION_UNSUPPORTED &&
-   
!acrtc_state->stream->link->psr_settings.psr_feature_enabled)
-   amdgpu_dm_link_setup_psr(acrtc_state->stream);
+   if (acrtc_state->update_type > UPDATE_TYPE_FAST) {
+   if 
(acrtc_state->stream->link->replay_settings.config.replay_supported &&
+   
!acrtc_state->stream->link->replay_settings.replay_feature_enabled) {
+   struct amdgpu_dm_connector *aconn =
+   (struct amdgpu_dm_connector 
*)acrtc_state->stream->dm_stream_context;
+   
amdgpu_dm_link_setup_replay(acrtc_state->stream->link, aconn);
+   } else if 
(acrtc_state->stream->link->psr_settings.psr_version != 
DC_PSR_VERSION_UNSUPPORTED &&
+   
!acrtc_state->stream->link->psr_settings.psr_feature_enabled) {
+   amdgpu_dm_link_setup_psr(acrtc_state->stream);
+   }
+   }
 
/* Decrement skip count when PSR is enabled and we're doing 
fast updates. */
if (acrtc_state->update_type == UPDATE_TYPE_FAST &&
@@ -8813,11 +8844,12 @@ static 

[PATCH 13/19] drm/amd/display: Add DP audio BW validation

[Alex Hung]

From: George Shen 

[Why]
Timings with small HBlank (such as CVT RBv2) can result in insufficient
HBlank bandwidth for audio SDP transmission when DSC is active. This
will cause some higher bandwidth audio modes to fail.

The combination of CVT RBv2 timings + DSC can commonly be encountered
in MST scenarios.

[How]
Add DP audio bandwidth validation for 8b/10b MST and 128b/132b SST/MST
cases and filter out modes that cannot be supported with the current
timing config.

Reviewed-by: Wenjing Liu 
Acked-by: Alex Hung 
Signed-off-by: George Shen 
---
 .../gpu/drm/amd/display/dc/dce/dce_audio.c| 288 +-
 .../gpu/drm/amd/display/dc/dce/dce_audio.h|   3 +-
 .../amd/display/dc/hwss/dce110/dce110_hwseq.c |  56 +++-
 drivers/gpu/drm/amd/display/dc/inc/hw/audio.h |   3 +-
 .../gpu/drm/amd/display/include/audio_types.h |  15 +
 5 files changed, 349 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c 
b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
index f0458b8f00af..07b507150c51 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
@@ -239,27 +239,289 @@ static void check_audio_bandwidth_hdmi(
}
}
 }
+static struct fixed31_32 get_link_symbol_clk_freq_mhz(enum dc_link_rate 
link_rate)
+{
+   switch (link_rate) {
+   case LINK_RATE_LOW:
+   return dc_fixpt_from_int(162); /* 162 MHz */
+   case LINK_RATE_HIGH:
+   return dc_fixpt_from_int(270); /* 270 MHz */
+   case LINK_RATE_HIGH2:
+   return dc_fixpt_from_int(540); /* 540 MHz */
+   case LINK_RATE_HIGH3:
+   return dc_fixpt_from_int(810); /* 810 MHz */
+   case LINK_RATE_UHBR10:
+   return dc_fixpt_from_fraction(3125, 10); /* 312.5 MHz */
+   case LINK_RATE_UHBR13_5:
+   return dc_fixpt_from_fraction(421875, 1000); /* 421.875 MHz */
+   case LINK_RATE_UHBR20:
+   return dc_fixpt_from_int(625); /* 625 MHz */
+   default:
+   /* Unexpected case, this requires debug if encountered. */
+   ASSERT(0);
+   return dc_fixpt_from_int(0);
+   }
+}
+
+struct dp_audio_layout_config {
+   uint8_t layouts_per_sample_denom;
+   uint8_t symbols_per_layout;
+   uint8_t max_layouts_per_audio_sdp;
+};
+
+static void get_audio_layout_config(
+   uint32_t channel_count,
+   enum dp_link_encoding encoding,
+   struct dp_audio_layout_config *output)
+{
+   /* Assuming L-PCM audio. Current implementation uses max 1 layout per 
SDP,
+* with each layout being the same size (8ch layout).
+*/
+   if (encoding == DP_8b_10b_ENCODING) {
+   if (channel_count == 2) {
+   output->layouts_per_sample_denom = 4;
+   output->symbols_per_layout = 40;
+   output->max_layouts_per_audio_sdp = 1;
+   } else if (channel_count == 8) {
+   output->layouts_per_sample_denom = 1;
+   output->symbols_per_layout = 40;
+   output->max_layouts_per_audio_sdp = 1;
+   }
+   } else if (encoding == DP_128b_132b_ENCODING) {
+   if (channel_count == 2) {
+   output->layouts_per_sample_denom = 4;
+   output->symbols_per_layout = 10;
+   output->max_layouts_per_audio_sdp = 1;
+   } else if (channel_count == 8) {
+   output->layouts_per_sample_denom = 1;
+   output->symbols_per_layout = 10;
+   output->max_layouts_per_audio_sdp = 1;
+   }
+   }
+}
 
-/*For DP SST, calculate if specified sample rates can fit into a given timing 
*/
-static void check_audio_bandwidth_dpsst(
+static uint32_t get_av_stream_map_lane_count(
+   enum dp_link_encoding encoding,
+   enum dc_lane_count lane_count,
+   bool is_mst)
+{
+   uint32_t av_stream_map_lane_count = 0;
+
+   if (encoding == DP_8b_10b_ENCODING) {
+   if (!is_mst)
+   av_stream_map_lane_count = lane_count;
+   else
+   av_stream_map_lane_count = 4;
+   } else if (encoding == DP_128b_132b_ENCODING) {
+   av_stream_map_lane_count = 4;
+   }
+
+   ASSERT(av_stream_map_lane_count != 0);
+
+   return av_stream_map_lane_count;
+}
+
+static uint32_t get_audio_sdp_overhead(
+   enum dp_link_encoding encoding,
+   enum dc_lane_count lane_count,
+   bool is_mst)
+{
+   uint32_t audio_sdp_overhead = 0;
+
+   if (encoding == DP_8b_10b_ENCODING) {
+   if (is_mst)
+   audio_sdp_overhead = 16; /* 4 * 2 + 8 */
+   else
+   audio_sdp_overhead = lane_count * 2 + 8;
+   } else if (encoding == DP_128b_132b_ENCODING) {
+   

[PATCH 12/19] drm/amd/display: Fix dml2 assigned pipe search

[Alex Hung]

From: Dmytro Laktyushkin 

[Why & How]
DML2 currently finds assigned pipes in array order rather than the
existing linked list order. This results in rearranging pipe order
on flip and more importantly otg inst and pipe idx mismatch.

This change preserves the order of existing pipes and guarantees
the head pipe will have matching otg inst and pipe idx.

Reviewed-by: Gabe Teeger 
Acked-by: Alex Hung 
Signed-off-by: Dmytro Laktyushkin 
---
 .../display/dc/dml2/dml2_dc_resource_mgmt.c   | 36 ++-
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_dc_resource_mgmt.c 
b/drivers/gpu/drm/amd/display/dc/dml2/dml2_dc_resource_mgmt.c
index 0baf39d64a2d..a0ce681b26c6 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_dc_resource_mgmt.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_dc_resource_mgmt.c
@@ -141,14 +141,28 @@ static unsigned int find_pipes_assigned_to_plane(struct 
dml2_context *ctx,
 {
int i;
unsigned int num_found = 0;
-   unsigned int plane_id_assigned_to_pipe;
+   unsigned int plane_id_assigned_to_pipe = -1;
 
for (i = 0; i < ctx->config.dcn_pipe_count; i++) {
-   if (state->res_ctx.pipe_ctx[i].plane_state && get_plane_id(ctx, 
state, state->res_ctx.pipe_ctx[i].plane_state,
-   state->res_ctx.pipe_ctx[i].stream->stream_id,
-   
ctx->v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_plane_index[state->res_ctx.pipe_ctx[i].pipe_idx],
 _id_assigned_to_pipe)) {
-   if (plane_id_assigned_to_pipe == plane_id)
-   pipes[num_found++] = i;
+   struct pipe_ctx *pipe = >res_ctx.pipe_ctx[i];
+
+   if (!pipe->stream)
+   continue;
+
+   get_plane_id(ctx, state, pipe->plane_state, 
pipe->stream->stream_id,
+   
ctx->v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_plane_index[pipe->pipe_idx],
+   _id_assigned_to_pipe);
+   if (pipe->plane_state && plane_id_assigned_to_pipe == plane_id 
&& !pipe->top_pipe && !pipe->prev_odm_pipe) {
+   while (pipe) {
+   struct pipe_ctx *mpo_pipe = pipe;
+
+   while (mpo_pipe) {
+   pipes[num_found++] = mpo_pipe->pipe_idx;
+   mpo_pipe = mpo_pipe->bottom_pipe;
+   }
+   pipe = pipe->next_odm_pipe;
+   }
+   break;
}
}
 
@@ -566,8 +580,14 @@ static unsigned int find_pipes_assigned_to_stream(struct 
dml2_context *ctx, stru
unsigned int num_found = 0;
 
for (i = 0; i < ctx->config.dcn_pipe_count; i++) {
-   if (state->res_ctx.pipe_ctx[i].stream && 
state->res_ctx.pipe_ctx[i].stream->stream_id == stream_id) {
-   pipes[num_found++] = i;
+   struct pipe_ctx *pipe = >res_ctx.pipe_ctx[i];
+
+   if (pipe->stream && pipe->stream->stream_id == stream_id && 
!pipe->top_pipe && !pipe->prev_odm_pipe) {
+   while (pipe) {
+   pipes[num_found++] = pipe->pipe_idx;
+   pipe = pipe->next_odm_pipe;
+   }
+   break;
}
}
 
-- 
2.34.1

[PATCH 11/19] drm/amd/display: Ensure populate uclk in bb construction

[Alex Hung]

From: Alvin Lee 

[Description]
- For some SKUs, the optimal DCFCLK for each UCLK is less than the
  smallest DCFCLK STA target due to low memory bandwidth. There is
  an assumption that the DCFCLK STA targets will always be less
  than one of the optimal DCFCLK values, but this is not true for
  SKUs that have low memory bandwidth. In this case we need to
  populate the optimal UCLK for each DCFCLK STA targets as the max
  UCLK freq.
- Also fix a bug in DML where start_state is not assigned and used
  correctly.

Reviewed-by: Samson Tam 
Reviewed-by: Chaitanya Dhere 
Acked-by: Alex Hung 
Signed-off-by: Alvin Lee 
---
 .../display/dc/dml/dcn30/display_mode_vba_30.c   | 16 
 .../drm/amd/display/dc/dml/dcn303/dcn303_fpu.c   | 11 +++
 .../display/dc/resource/dcn30/dcn30_resource.c   | 11 +++
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
index 63c48c29ba49..e7f4a2d491cc 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
@@ -4273,7 +4273,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
 
//Calculate Swath, DET Configuration, DCFCLKDeepSleep
//
-   for (i = 0; i < mode_lib->soc.num_states; ++i) {
+   for (i = start_state; i < mode_lib->soc.num_states; ++i) {
for (j = 0; j <= 1; ++j) {
for (k = 0; k < v->NumberOfActivePlanes; ++k) {
v->RequiredDPPCLKThisState[k] = 
v->RequiredDPPCLK[i][j][k];
@@ -4576,7 +4576,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
 
//Calculate Return BW
 
-   for (i = 0; i < mode_lib->soc.num_states; ++i) {
+   for (i = start_state; i < mode_lib->soc.num_states; ++i) {
for (j = 0; j <= 1; ++j) {
for (k = 0; k <= v->NumberOfActivePlanes - 1; k++) {
if (v->BlendingAndTiming[k] == k) {
@@ -4635,7 +4635,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l

v->UrgentOutOfOrderReturnPerChannelVMDataOnly);
v->FinalDRAMClockChangeLatency = (v->DRAMClockChangeLatencyOverride > 0 
? v->DRAMClockChangeLatencyOverride : v->DRAMClockChangeLatency);
 
-   for (i = 0; i < mode_lib->soc.num_states; ++i) {
+   for (i = start_state; i < mode_lib->soc.num_states; ++i) {
for (j = 0; j <= 1; ++j) {
v->DCFCLKState[i][j] = v->DCFCLKPerState[i];
}
@@ -4646,7 +4646,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
 
if (v->ClampMinDCFCLK) {
/* Clamp calculated values to actual minimum */
-   for (i = 0; i < mode_lib->soc.num_states; ++i) {
+   for (i = start_state; i < mode_lib->soc.num_states; 
++i) {
for (j = 0; j <= 1; ++j) {
if (v->DCFCLKState[i][j] < 
mode_lib->soc.min_dcfclk) {
v->DCFCLKState[i][j] = 
mode_lib->soc.min_dcfclk;
@@ -4656,7 +4656,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
}
}
 
-   for (i = 0; i < mode_lib->soc.num_states; ++i) {
+   for (i = start_state; i < mode_lib->soc.num_states; ++i) {
for (j = 0; j <= 1; ++j) {
v->IdealSDPPortBandwidthPerState[i][j] = dml_min3(
v->ReturnBusWidth * 
v->DCFCLKState[i][j],
@@ -4674,7 +4674,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
 
//Re-ordering Buffer Support Check
 
-   for (i = 0; i < mode_lib->soc.num_states; ++i) {
+   for (i = start_state; i < mode_lib->soc.num_states; ++i) {
for (j = 0; j <= 1; ++j) {
if ((v->ROBBufferSizeInKByte - 
v->PixelChunkSizeInKByte) * 1024 / v->ReturnBWPerState[i][j]
> (v->RoundTripPingLatencyCycles + 32) 
/ v->DCFCLKState[i][j] + ReorderingBytes / v->ReturnBWPerState[i][j]) {
@@ -4692,7 +4692,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
MaxTotalVActiveRDBandwidth = MaxTotalVActiveRDBandwidth + 
v->ReadBandwidthLuma[k] + v->ReadBandwidthChroma[k];
}
 
-   for (i = 0; i < mode_lib->soc.num_states; ++i) {
+   for (i = start_state; i < mode_lib->soc.num_states; ++i) {
for (j = 0; j <= 1; ++j) {
v->MaxTotalVerticalActiveAvailableBandwidth[i][j] = 
dml_min(

[PATCH 10/19] drm/amd/display: Update P010 scaling cap

[Alex Hung]

From: Charlene Liu 

[Why]
Keep the same as previous APU and also insert clock dump

Reviewed-by: Ovidiu Bunea 
Acked-by: Alex Hung 
Signed-off-by: Charlene Liu 
---
 .../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c  | 25 +--
 .../dc/resource/dcn35/dcn35_resource.c|  2 +-
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
index 9c660d1facc7..0e5a3184f01c 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
@@ -384,19 +384,6 @@ static void dcn35_enable_pme_wa(struct clk_mgr 
*clk_mgr_base)
dcn35_smu_enable_pme_wa(clk_mgr);
 }
 
-void dcn35_init_clocks(struct clk_mgr *clk_mgr)
-{
-   uint32_t ref_dtbclk = clk_mgr->clks.ref_dtbclk_khz;
-
-   memset(&(clk_mgr->clks), 0, sizeof(struct dc_clocks));
-
-   // Assumption is that boot state always supports pstate
-   clk_mgr->clks.ref_dtbclk_khz = ref_dtbclk;  // restore ref_dtbclk
-   clk_mgr->clks.p_state_change_support = true;
-   clk_mgr->clks.prev_p_state_change_support = true;
-   clk_mgr->clks.pwr_state = DCN_PWR_STATE_UNKNOWN;
-   clk_mgr->clks.zstate_support = DCN_ZSTATE_SUPPORT_UNKNOWN;
-}
 
 bool dcn35_are_clock_states_equal(struct dc_clocks *a,
struct dc_clocks *b)
@@ -421,7 +408,19 @@ static void dcn35_dump_clk_registers(struct 
clk_state_registers_and_bypass *regs
struct clk_mgr_dcn35 *clk_mgr)
 {
 }
+void dcn35_init_clocks(struct clk_mgr *clk_mgr)
+{
+   uint32_t ref_dtbclk = clk_mgr->clks.ref_dtbclk_khz;
 
+   memset(&(clk_mgr->clks), 0, sizeof(struct dc_clocks));
+
+   // Assumption is that boot state always supports pstate
+   clk_mgr->clks.ref_dtbclk_khz = ref_dtbclk;  // restore ref_dtbclk
+   clk_mgr->clks.p_state_change_support = true;
+   clk_mgr->clks.prev_p_state_change_support = true;
+   clk_mgr->clks.pwr_state = DCN_PWR_STATE_UNKNOWN;
+   clk_mgr->clks.zstate_support = DCN_ZSTATE_SUPPORT_UNKNOWN;
+}
 static struct clk_bw_params dcn35_bw_params = {
.vram_type = Ddr4MemType,
.num_channels = 1,
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c 
b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
index 761ec9891875..7d8e957d6a19 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
@@ -701,7 +701,7 @@ static const struct dc_plane_cap plane_cap = {
 
// 6:1 downscaling ratio: 1000/6 = 166.666
.max_downscale_factor = {
-   .argb = 167,
+   .argb = 250,
.nv12 = 167,
.fp16 = 167
},
-- 
2.34.1

[PATCH 09/19] drm/amd/display: Reenable windowed mpo odm support

[Alex Hung]

From: Wenjing Liu 

[Why]
The feature was disabled due to regression found during testing. Now
that all the pending issues are addressed, we are reenabling the power
saving feature again.

The feature optimizes dispclk level when user is using MPO capable
broswers or watching MPO capable videos in windowed mode. The feature
achieves power optimization by utilizing free pipes to process incoming
pixels in parallel. So it reduces max dispclk requirements for each
pipe.

Previously ODM power optimization will be disabled when MPO plane is
present due to technical challeges. This is mainly because ODM divides
pixel workload with respect to stream but MPO plane position and size
are arbitrary with respect to stream. The pixel processing workload of
an MPO plane is not guaranteed to be evenly distributed across DCN pipes.
For example if a plane is moved inside single ODM slice, all the
processing for the plane is distributed to the pipe in the current ODM
slice, while the other ODM slices don't need to process this plane. If
the plane is then moved to the middle crosing two ODM slices, each ODM
slice gets half of the workload. This is especially difficult when the
plane itself has a large source rect which can't be processed by single
DCN pipe. In this case we can't enable ODM power optimization when the
plane is only within one ODM slice.

[How]
To overcome the challeges, new pipe resource management is in place to
make sure a plane is validated with ODM power optimization support if
it can be validated regardless of its position and the same pipe
topology can be used regardless of the plane's position. When the plane
is moved outside current ODM slice, we will set recout to 0 so the pipe
can be idling without the need to update pipe topology. When the user
resizes a plane, it may result in downscaling ratio changes. When the
downscaling ratio is above single pipe's threshold, we will seamlessly
exit ODM power optimization and applies MPC combine to support the plane.
when downscaling ratio becomes smaller, we will seamlessly enter ODM
power optimization again. All these pipe transitions happen
automatically and quietly when the conditions are met without any visual
impacts to the user.

Reviewed-by: Martin Leung 
Acked-by: Alex Hung 
Signed-off-by: Wenjing Liu 
---
 drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c   | 1 +
 drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c 
b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
index c4d71e7f18af..5c24a769ce60 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
@@ -2104,6 +2104,7 @@ static bool dcn32_resource_construct(
dc->config.use_pipe_ctx_sync_logic = true;
 
dc->config.dc_mode_clk_limit_support = true;
+   dc->config.enable_windowed_mpo_odm = true;
/* read VBIOS LTTPR caps */
{
if (ctx->dc_bios->funcs->get_lttpr_caps) {
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c 
b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
index 74412e5f03fe..b356fed1726d 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
@@ -1760,6 +1760,7 @@ static bool dcn321_resource_construct(
dc->caps.color.mpc.ocsc = 1;
 
dc->config.dc_mode_clk_limit_support = true;
+   dc->config.enable_windowed_mpo_odm = true;
/* read VBIOS LTTPR caps */
{
if (ctx->dc_bios->funcs->get_lttpr_caps) {
-- 
2.34.1

[PATCH 08/19] drm/amd/display: Fix DML2 watermark calculation

[Alex Hung]

From: Ovidiu Bunea 

[Why]
core_mode_programming in DML2 should output watermark calculations
to locals, but it incorrectly uses mode_lib

[How]
update code to match HW DML2

Cc: Mario Limonciello 
Cc: Alex Deucher 
Cc: sta...@vger.kernel.org
Reviewed-by: Charlene Liu 
Acked-by: Alex Hung 
Signed-off-by: Ovidiu Bunea 
---
 .../drm/amd/display/dc/dml2/display_mode_core.c| 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/display_mode_core.c 
b/drivers/gpu/drm/amd/display/dc/dml2/display_mode_core.c
index a6b938a12de1..9be5ebf3a8c0 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/display_mode_core.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/display_mode_core.c
@@ -9446,13 +9446,13 @@ void dml_core_mode_programming(struct 
display_mode_lib_st *mode_lib, const struc
CalculateWatermarks_params->CompressedBufferSizeInkByte = 
locals->CompressedBufferSizeInkByte;
 
// Output
-   CalculateWatermarks_params->Watermark = >dummy_watermark; // 
Watermarks *Watermark
-   CalculateWatermarks_params->DRAMClockChangeSupport = 
_lib->ms.support.DRAMClockChangeSupport[0];
-   
CalculateWatermarks_params->MaxActiveDRAMClockChangeLatencySupported = 
>dummy_single_array[0][0]; // dml_float_t 
*MaxActiveDRAMClockChangeLatencySupported[]
-   CalculateWatermarks_params->SubViewportLinesNeededInMALL = 
_lib->ms.SubViewportLinesNeededInMALL[j]; // dml_uint_t 
SubViewportLinesNeededInMALL[]
-   CalculateWatermarks_params->FCLKChangeSupport = 
_lib->ms.support.FCLKChangeSupport[0];
-   CalculateWatermarks_params->MaxActiveFCLKChangeLatencySupported 
= >dummy_single[0]; // dml_float_t *MaxActiveFCLKChangeLatencySupported
-   CalculateWatermarks_params->USRRetrainingSupport = 
_lib->ms.support.USRRetrainingSupport[0];
+   CalculateWatermarks_params->Watermark = >Watermark; // 
Watermarks *Watermark
+   CalculateWatermarks_params->DRAMClockChangeSupport = 
>DRAMClockChangeSupport;
+   
CalculateWatermarks_params->MaxActiveDRAMClockChangeLatencySupported = 
locals->MaxActiveDRAMClockChangeLatencySupported; // dml_float_t 
*MaxActiveDRAMClockChangeLatencySupported[]
+   CalculateWatermarks_params->SubViewportLinesNeededInMALL = 
locals->SubViewportLinesNeededInMALL; // dml_uint_t 
SubViewportLinesNeededInMALL[]
+   CalculateWatermarks_params->FCLKChangeSupport = 
>FCLKChangeSupport;
+   CalculateWatermarks_params->MaxActiveFCLKChangeLatencySupported 
= >MaxActiveFCLKChangeLatencySupported; // dml_float_t 
*MaxActiveFCLKChangeLatencySupported
+   CalculateWatermarks_params->USRRetrainingSupport = 
>USRRetrainingSupport;
 
CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport(
_lib->scratch,
-- 
2.34.1

[PATCH 07/19] drm/amd/display: Clear OPTC mem select on disable

[Alex Hung]

From: Ilya Bakoulin 

[Why]
Not clearing the memory select bits prior to OPTC disable can cause DSC
corruption issues when attempting to reuse a memory instance for another
OPTC that enables ODM.

[How]
Clear the memory select bits prior to disabling an OPTC.

Cc: Mario Limonciello 
Cc: Alex Deucher 
Cc: sta...@vger.kernel.org
Reviewed-by: Charlene Liu 
Acked-by: Alex Hung 
Signed-off-by: Ilya Bakoulin 
---
 drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c | 3 +++
 drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c 
b/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c
index 1788eb29474b..823493543325 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c
@@ -173,6 +173,9 @@ static bool optc32_disable_crtc(struct timing_generator 
*optc)
OPTC_SEG3_SRC_SEL, 0xf,
OPTC_NUM_OF_INPUT_SEGMENT, 0);
 
+   REG_UPDATE(OPTC_MEMORY_CONFIG,
+   OPTC_MEM_SEL, 0);
+
/* disable otg request until end of the first line
 * in the vertical blank region
 */
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c 
b/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
index 3d6c1b2c2b4d..5b1547508850 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
@@ -145,6 +145,9 @@ static bool optc35_disable_crtc(struct timing_generator 
*optc)
OPTC_SEG3_SRC_SEL, 0xf,
OPTC_NUM_OF_INPUT_SEGMENT, 0);
 
+   REG_UPDATE(OPTC_MEMORY_CONFIG,
+   OPTC_MEM_SEL, 0);
+
/* disable otg request until end of the first line
 * in the vertical blank region
 */
-- 
2.34.1

[PATCH 06/19] drm/amd/display: Floor to mhz when requesting dpp disp clock changes to SMU

[Alex Hung]

From: Wenjing Liu 

[Why]
SMU uses discrete dpp and disp clock levels. When we submit SMU request
for clock changes in Mhz we need to floor the requested value from Khz so
SMU will choose the next higher clock level in Khz to set. If we ceil to
Mhz, SMU will have to choose the next higher clock level after the ceil,
which could result in unnecessarily jumpping to the next level.

For example, we request 1911,111Khz which is exactly one of the SMU preset
level. If we pass 1912Mhz, SMU will choose 2150,000 khz. If we pass
1911Mhz, SMU will choose 1911,111kHz, which is the expected value.

Reviewed-by: Alvin Lee 
Acked-by: Alex Hung 
Signed-off-by: Wenjing Liu 
---
 .../display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c  | 40 +--
 .../amd/display/dc/inc/hw/clk_mgr_internal.h  |  5 +++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c
index aadd07bc68c5..8fa0aae941c3 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c
@@ -387,7 +387,15 @@ static void dcn32_update_clocks_update_dentist(
uint32_t temp_dispclk_khz = (DENTIST_DIVIDER_RANGE_SCALE_FACTOR 
* clk_mgr->base.dentist_vco_freq_khz) / temp_disp_divider;
 
if (clk_mgr->smu_present)
-   dcn32_smu_set_hard_min_by_freq(clk_mgr, PPCLK_DISPCLK, 
khz_to_mhz_ceil(temp_dispclk_khz));
+   /*
+* SMU uses discrete dispclk presets. We applied
+* the same formula to increase our dppclk_khz
+* to the next matching discrete value. By
+* contract, we should use the preset dispclk
+* floored in Mhz to describe the intended clock.
+*/
+   dcn32_smu_set_hard_min_by_freq(clk_mgr, PPCLK_DISPCLK,
+   khz_to_mhz_floor(temp_dispclk_khz));
 
if (dc->debug.override_dispclk_programming) {
REG_GET(DENTIST_DISPCLK_CNTL,
@@ -426,7 +434,15 @@ static void dcn32_update_clocks_update_dentist(
 
/* do requested DISPCLK updates*/
if (clk_mgr->smu_present)
-   dcn32_smu_set_hard_min_by_freq(clk_mgr, PPCLK_DISPCLK, 
khz_to_mhz_ceil(clk_mgr->base.clks.dispclk_khz));
+   /*
+* SMU uses discrete dispclk presets. We applied
+* the same formula to increase our dppclk_khz
+* to the next matching discrete value. By
+* contract, we should use the preset dispclk
+* floored in Mhz to describe the intended clock.
+*/
+   dcn32_smu_set_hard_min_by_freq(clk_mgr, PPCLK_DISPCLK,
+   
khz_to_mhz_floor(clk_mgr->base.clks.dispclk_khz));
 
if (dc->debug.override_dispclk_programming) {
REG_GET(DENTIST_DISPCLK_CNTL,
@@ -734,7 +750,15 @@ static void dcn32_update_clocks(struct clk_mgr 
*clk_mgr_base,
clk_mgr_base->clks.dppclk_khz = new_clocks->dppclk_khz;
 
if (clk_mgr->smu_present && !dpp_clock_lowered)
-   dcn32_smu_set_hard_min_by_freq(clk_mgr, PPCLK_DPPCLK, 
khz_to_mhz_ceil(clk_mgr_base->clks.dppclk_khz));
+   /*
+* SMU uses discrete dppclk presets. We applied
+* the same formula to increase our dppclk_khz
+* to the next matching discrete value. By
+* contract, we should use the preset dppclk
+* floored in Mhz to describe the intended clock.
+*/
+   dcn32_smu_set_hard_min_by_freq(clk_mgr, PPCLK_DPPCLK,
+   
khz_to_mhz_floor(clk_mgr_base->clks.dppclk_khz));
 
update_dppclk = true;
}
@@ -765,7 +789,15 @@ static void dcn32_update_clocks(struct clk_mgr 
*clk_mgr_base,
dcn32_update_clocks_update_dpp_dto(clk_mgr, context, 
safe_to_lower);
dcn32_update_clocks_update_dentist(clk_mgr, context);
if (clk_mgr->smu_present)
-   dcn32_smu_set_hard_min_by_freq(clk_mgr, 
PPCLK_DPPCLK, khz_to_mhz_ceil(clk_mgr_base->clks.dppclk_khz));
+   /*
+* SMU uses discrete dppclk presets. We applied
+* the same formula to increase our dppclk_khz
+* to the next matching discrete value. By
+* contract, we should use the preset dppclk
+* floored in Mhz to describe the intended 
clock.
+*/
+  

[PATCH 05/19] drm/amd/display: Port DENTIST hang and TDR fixes to OTG disable W/A

[Alex Hung]

From: Nicholas Kazlauskas 

[Why]
We can experience DENTIST hangs during optimize_bandwidth or TDRs if
FIFO is toggled and hangs.

[How]
Port the DCN35 fixes to DCN314.

Cc: Mario Limonciello 
Cc: Alex Deucher 
Cc: sta...@vger.kernel.org
Reviewed-by: Charlene Liu 
Acked-by: Alex Hung 
Signed-off-by: Nicholas Kazlauskas 
---
 .../dc/clk_mgr/dcn314/dcn314_clk_mgr.c| 21 ---
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_clk_mgr.c
index 878c0e7b78ab..a84f1e376dee 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_clk_mgr.c
@@ -145,30 +145,27 @@ static int dcn314_get_active_display_cnt_wa(
return display_count;
 }
 
-static void dcn314_disable_otg_wa(struct clk_mgr *clk_mgr_base, struct 
dc_state *context, bool disable)
+static void dcn314_disable_otg_wa(struct clk_mgr *clk_mgr_base, struct 
dc_state *context,
+ bool safe_to_lower, bool disable)
 {
struct dc *dc = clk_mgr_base->ctx->dc;
int i;
 
for (i = 0; i < dc->res_pool->pipe_count; ++i) {
-   struct pipe_ctx *pipe = >current_state->res_ctx.pipe_ctx[i];
+   struct pipe_ctx *pipe = safe_to_lower
+   ? >res_ctx.pipe_ctx[i]
+   : >current_state->res_ctx.pipe_ctx[i];
 
if (pipe->top_pipe || pipe->prev_odm_pipe)
continue;
if (pipe->stream && (pipe->stream->dpms_off || 
dc_is_virtual_signal(pipe->stream->signal))) {
-   struct stream_encoder *stream_enc = 
pipe->stream_res.stream_enc;
-
if (disable) {
-   if (stream_enc && 
stream_enc->funcs->disable_fifo)
-   
pipe->stream_res.stream_enc->funcs->disable_fifo(stream_enc);
+   if (pipe->stream_res.tg && 
pipe->stream_res.tg->funcs->immediate_disable_crtc)
+   
pipe->stream_res.tg->funcs->immediate_disable_crtc(pipe->stream_res.tg);
 
-   
pipe->stream_res.tg->funcs->immediate_disable_crtc(pipe->stream_res.tg);
reset_sync_context_for_pipe(dc, context, i);
} else {

pipe->stream_res.tg->funcs->enable_crtc(pipe->stream_res.tg);
-
-   if (stream_enc && 
stream_enc->funcs->enable_fifo)
-   
pipe->stream_res.stream_enc->funcs->enable_fifo(stream_enc);
}
}
}
@@ -297,11 +294,11 @@ void dcn314_update_clocks(struct clk_mgr *clk_mgr_base,
}
 
if (should_set_clock(safe_to_lower, new_clocks->dispclk_khz, 
clk_mgr_base->clks.dispclk_khz)) {
-   dcn314_disable_otg_wa(clk_mgr_base, context, true);
+   dcn314_disable_otg_wa(clk_mgr_base, context, safe_to_lower, 
true);
 
clk_mgr_base->clks.dispclk_khz = new_clocks->dispclk_khz;
dcn314_smu_set_dispclk(clk_mgr, clk_mgr_base->clks.dispclk_khz);
-   dcn314_disable_otg_wa(clk_mgr_base, context, false);
+   dcn314_disable_otg_wa(clk_mgr_base, context, safe_to_lower, 
false);
 
update_dispclk = true;
}
-- 
2.34.1

[PATCH 04/19] drm/amd/display: Add logging resource checks

[Alex Hung]

From: Charlene Liu 

[Why]
When mapping resources, resources could be unavailable.

Cc: Mario Limonciello 
Cc: Alex Deucher 
Cc: sta...@vger.kernel.org
Reviewed-by: Sung joon Kim 
Acked-by: Alex Hung 
Signed-off-by: Charlene Liu 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c  | 4 +++-
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 4 
 drivers/gpu/drm/amd/display/dc/core/dc_state.c| 5 +++--
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 69e726630241..aa7c02ba948e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -3522,7 +3522,7 @@ static void commit_planes_for_stream(struct dc *dc,
top_pipe_to_program = resource_get_otg_master_for_stream(
>res_ctx,
stream);
-
+   ASSERT(top_pipe_to_program != NULL);
for (i = 0; i < dc->res_pool->pipe_count; i++) {
struct pipe_ctx *old_pipe = 
>current_state->res_ctx.pipe_ctx[i];
 
@@ -4345,6 +4345,8 @@ static bool 
should_commit_minimal_transition_for_windowed_mpo_odm(struct dc *dc,
 
cur_pipe = 
resource_get_otg_master_for_stream(>current_state->res_ctx, stream);
new_pipe = resource_get_otg_master_for_stream(>res_ctx, 
stream);
+   if (!cur_pipe || !new_pipe)
+   return false;
cur_is_odm_in_use = resource_get_odm_slice_count(cur_pipe) > 1;
new_is_odm_in_use = resource_get_odm_slice_count(new_pipe) > 1;
if (cur_is_odm_in_use == new_is_odm_in_use)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index f2abc1096ffb..9fbdb09697fd 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2194,6 +2194,10 @@ void resource_log_pipe_topology_update(struct dc *dc, 
struct dc_state *state)
for (stream_idx = 0; stream_idx < state->stream_count; stream_idx++) {
otg_master = resource_get_otg_master_for_stream(
>res_ctx, state->streams[stream_idx]);
+   if (!otg_master || otg_master->stream_res.tg == NULL) {
+   DC_LOG_DC("topology update: otg_master NULL stream_idx 
%d!\n", stream_idx);
+   return;
+   }
slice_count = resource_get_opp_heads_for_otg_master(otg_master,
>res_ctx, opp_heads);
for (slice_idx = 0; slice_idx < slice_count; slice_idx++) {
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
index 56feee0ff01b..88c6436b28b6 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
@@ -434,8 +434,9 @@ bool dc_state_add_plane(
 
otg_master_pipe = resource_get_otg_master_for_stream(
>res_ctx, stream);
-   added = resource_append_dpp_pipes_for_plane_composition(state,
-   dc->current_state, pool, otg_master_pipe, plane_state);
+   if (otg_master_pipe)
+   added = resource_append_dpp_pipes_for_plane_composition(state,
+   dc->current_state, pool, otg_master_pipe, 
plane_state);
 
if (added) {
stream_status->plane_states[stream_status->plane_count] =
-- 
2.34.1

[PATCH 03/19] drm/amd/display: Add Replay IPS register for DMUB command table

[Alex Hung]

From: Alvin Lee 

- Introduce a new Replay mode for DMUB version 0.0.199.0

Reviewed-by: Martin Leung 
Acked-by: Alex Hung 
Signed-off-by: Alvin Lee 
---
 drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h 
b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
index c64b6c848ef7..bcd3c361cca5 100644
--- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
+++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
@@ -2832,6 +2832,7 @@ struct dmub_rb_cmd_psr_set_power_opt {
 #define REPLAY_RESIDENCY_MODE_MASK (0x1 << 
REPLAY_RESIDENCY_MODE_SHIFT)
 # define REPLAY_RESIDENCY_MODE_PHY (0x0 << 
REPLAY_RESIDENCY_MODE_SHIFT)
 # define REPLAY_RESIDENCY_MODE_ALPM(0x1 << 
REPLAY_RESIDENCY_MODE_SHIFT)
+# define REPLAY_RESIDENCY_MODE_IPS 0x10
 
 #define REPLAY_RESIDENCY_ENABLE_MASK   (0x1 << 
REPLAY_RESIDENCY_ENABLE_SHIFT)
 # define REPLAY_RESIDENCY_DISABLE  (0x0 << 
REPLAY_RESIDENCY_ENABLE_SHIFT)
-- 
2.34.1

[PATCH 02/19] drm/amd/display: Init link enc resources in dc_state only if res_pool presents

[Alex Hung]

From: Dillon Varone 

[Why & How]
res_pool is not initialized in all situations such as virtual
environments, and therefore link encoder resources should not be
initialized if res_pool is NULL.

Cc: Mario Limonciello 
Cc: Alex Deucher 
Cc: sta...@vger.kernel.org
Reviewed-by: Martin Leung 
Acked-by: Alex Hung 
Signed-off-by: Dillon Varone 
---
 drivers/gpu/drm/amd/display/dc/core/dc_state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
index 460a8010c79f..56feee0ff01b 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
@@ -267,7 +267,8 @@ void dc_state_construct(struct dc *dc, struct dc_state 
*state)
state->clk_mgr = dc->clk_mgr;
 
/* Initialise DIG link encoder resource tracking variables. */
-   link_enc_cfg_init(dc, state);
+   if (dc->res_pool)
+   link_enc_cfg_init(dc, state);
 }
 
 void dc_state_destruct(struct dc_state *state)
-- 
2.34.1

[PATCH 01/19] drm/amd/display: Allow IPS2 during Replay

[Alex Hung]

From: Nicholas Kazlauskas 

[Why & How]
Add regkey to block video playback in IPS2 by default

Allow idle optimizations in the same spot we allow Replay for
video playback usecases.

Avoid sending it when there's an external display connected by
modifying the allow idle checks to check for active non-eDP screens.

Reviewed-by: Charlene Liu 
Acked-by: Alex Hung 
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 2 ++
 drivers/gpu/drm/amd/display/dc/dc.h | 1 +
 drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c | 9 -
 3 files changed, 11 insertions(+), 1 deletion(-)

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 111c6f51f0ae..95ff3800fc87 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1716,6 +1716,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
init_data.flags.disable_ips = DMUB_IPS_DISABLE_ALL;
 
+   init_data.flags.disable_ips_in_vpb = 1;
+
/* Enable DWB for tested platforms only */
if (amdgpu_ip_version(adev, DCE_HWIP, 0) >= IP_VERSION(3, 0, 0))
init_data.num_virtual_links = 1;
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 5d7aa882416b..c9317ea0258e 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -434,6 +434,7 @@ struct dc_config {
bool EnableMinDispClkODM;
bool enable_auto_dpm_test_logs;
unsigned int disable_ips;
+   unsigned int disable_ips_in_vpb;
 };
 
 enum visual_confirm {
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
index 9c806385ecbd..8b6c49622f3b 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
@@ -680,7 +680,7 @@ void dcn35_power_down_on_boot(struct dc *dc)
 bool dcn35_apply_idle_power_optimizations(struct dc *dc, bool enable)
 {
struct dc_link *edp_links[MAX_NUM_EDP];
-   int edp_num;
+   int i, edp_num;
if (dc->debug.dmcub_emulation)
return true;
 
@@ -688,6 +688,13 @@ bool dcn35_apply_idle_power_optimizations(struct dc *dc, 
bool enable)
dc_get_edp_links(dc, edp_links, _num);
if (edp_num == 0 || edp_num > 1)
return false;
+
+   for (i = 0; i < dc->current_state->stream_count; ++i) {
+   struct dc_stream_state *stream = 
dc->current_state->streams[i];
+
+   if (!stream->dpms_off && 
!dc_is_embedded_signal(stream->signal))
+   return false;
+   }
}
 
// TODO: review other cases when idle optimization is allowed
-- 
2.34.1

[PATCH 00/19] DC Patches January 10, 2024

[Alex Hung]

This DC patchset brings improvements in multiple areas. In summary, we 
highlight:

* Fixes on DCN35 and DML2.
* Enhancements in DMUB.
* Improvements on IPS, DP and MPO and others.

Cc: Daniel Wheeler 

Alvin Lee (2):
  drm/amd/display: Add Replay IPS register for DMUB command table
  drm/amd/display: Ensure populate uclk in bb construction

Charlene Liu (2):
  drm/amd/display: Add logging resource checks
  drm/amd/display: Update P010 scaling cap

Dillon Varone (1):
  drm/amd/display: Init link enc resources in dc_state only if res_pool
presents

Dmytro Laktyushkin (1):
  drm/amd/display: Fix dml2 assigned pipe search

George Shen (1):
  drm/amd/display: Add DP audio BW validation

Ilya Bakoulin (1):
  drm/amd/display: Clear OPTC mem select on disable

Martin Leung (1):
  drm/amd/display: 3.2.267

Nicholas Kazlauskas (5):
  drm/amd/display: Allow IPS2 during Replay
  drm/amd/display: Port DENTIST hang and TDR fixes to OTG disable W/A
  drm/amd/display: Rework DC Z10 restore
  drm/amd/display: Set default Z8 minimum residency for DCN35
  drm/amd/display: Allow Z8 for multiplane configurations on DCN35

Ovidiu Bunea (1):
  drm/amd/display: Fix DML2 watermark calculation

Tom Chung (1):
  drm/amd/display: Enable Panel Replay for static screen use case

Wayne Lin (1):
  drm/amd/display: Align the returned error code with legacy DP

Wenjing Liu (2):
  drm/amd/display: Floor to mhz when requesting dpp disp clock changes
to SMU
  drm/amd/display: Reenable windowed mpo odm support

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  44 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c|  59 +++-
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |   5 +
 .../amd/display/amdgpu_dm/amdgpu_dm_replay.c  | 119 +---
 .../amd/display/amdgpu_dm/amdgpu_dm_replay.h  |   4 +-
 .../dc/clk_mgr/dcn314/dcn314_clk_mgr.c|  21 +-
 .../display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c  |  40 ++-
 .../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c  |  25 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  15 +-
 .../gpu/drm/amd/display/dc/core/dc_resource.c |   4 +
 .../gpu/drm/amd/display/dc/core/dc_state.c|   8 +-
 .../gpu/drm/amd/display/dc/core/dc_stream.c   |   9 +-
 drivers/gpu/drm/amd/display/dc/dc.h   |   3 +-
 .../gpu/drm/amd/display/dc/dce/dce_audio.c| 288 +-
 .../gpu/drm/amd/display/dc/dce/dce_audio.h|   3 +-
 .../dc/dml/dcn30/display_mode_vba_30.c|  16 +-
 .../amd/display/dc/dml/dcn303/dcn303_fpu.c|  11 +
 .../drm/amd/display/dc/dml/dcn35/dcn35_fpu.c  |   4 +-
 .../amd/display/dc/dml2/display_mode_core.c   |  14 +-
 .../display/dc/dml2/dml2_dc_resource_mgmt.c   |  36 ++-
 .../amd/display/dc/hwss/dce110/dce110_hwseq.c |  56 +++-
 .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c   |  11 +-
 drivers/gpu/drm/amd/display/dc/inc/hw/audio.h |   3 +-
 .../amd/display/dc/inc/hw/clk_mgr_internal.h  |   5 +
 .../amd/display/dc/optc/dcn32/dcn32_optc.c|   3 +
 .../amd/display/dc/optc/dcn35/dcn35_optc.c|   3 +
 .../dc/resource/dcn30/dcn30_resource.c|  11 +
 .../dc/resource/dcn32/dcn32_resource.c|   1 +
 .../dc/resource/dcn321/dcn321_resource.c  |   1 +
 .../dc/resource/dcn35/dcn35_resource.c|   3 +-
 .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   |   1 +
 .../gpu/drm/amd/display/include/audio_types.h |  15 +
 32 files changed, 689 insertions(+), 152 deletions(-)

-- 
2.34.1

RE: Documentation for RGB strip on RX 7900 XTX (Reference)

[Deucher, Alexander]

[Public]

> -Original Message-
> From: amd-gfx  On Behalf Of
> Alexander Koskovich
> Sent: Tuesday, January 9, 2024 9:21 PM
> To: Christian König 
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: Documentation for RGB strip on RX 7900 XTX (Reference)
>
> Are there any userspace utilities for checking out the ATOMBIOS tables? Have
> never done so and all the utilities I've found online are too old for this 
> card (at
> least it refuses to open the VBIOS for this card).


There is atomdis and I think there is some limited bios parsing in umr. That 
said, you'd need probably need to update the headers in those projects with the 
latest ones from the kernel kernel and probably add new parsers for the newer 
data table versions.

Alex

>
>
> On Tuesday, January 9th, 2024 at 3:02 AM, Christian König
>  wrote:
>
>
> >
> >
> > Am 08.01.24 um 23:32 schrieb Deucher, Alexander:
> >
> > > [Public]
> > >
> > > > -Original Message-
> > > > From: amd-gfx amd-gfx-boun...@lists.freedesktop.org On Behalf Of
> > > > Alexander Koskovich
> > > > Sent: Sunday, January 7, 2024 11:19 PM
> > > > To: amd-gfx@lists.freedesktop.org
> > > > Subject: Documentation for RGB strip on RX 7900 XTX (Reference)
> > > >
> > > > Hello,
> > > >
> > > > I was wondering if AMD would be able provide any documentation for
> > > > the RGB strip on the reference cooler
> > > > (https://www.amd.com/en/products/graphics/amd-radeon-rx-
> 7900xtx)?
> > > > It looks to be handled via I2C commands to the SMU, but having
> > > > proper documentation would be extremely helpful.
> > > > It depends on the AIB/OEM and how they designed the specific board.
> The RGB controller will either be attached to the DDCVGA i2c bus on the
> display hardware or the second SMU i2c bus. The former will require changes
> to the amdgpu display code to register display i2c buses that are not used by
> the display connectors on the board so they can be used by 3rd party
> applications. Currently we only register i2c buses used for display 
> connectors.
> The latter buses are already registered with the i2c subsystem since they are
> used for other things like EEPROMs on server and workstation cards and
> should be available via standard Linux i2c APIs. I'm not sure what i2c LED
> controllers each AIB vendor uses off hand. https://openrgb.org/index.html
> would probably be a good resource for that information.
> >
> >
> >
> > It might also be a good idea to look some of the ATOMBIOS tables found
> > on your device.
> >
> > Those tables are filled in by the AIB/OEM with the information which
> > connectors (HDMI, DVI, DP etc...) are on the board and I bet that the
> > information which RGB controller is used and where to find it is
> > somewhere in there as well.
> >
> > Adding Harry from our display team, might be that he has some more
> > hints as well.
> >
> > Christian.
> >
> > > Alex

[PATCH] drm/amd/display: Avoid enum conversion warning

[Nathan Chancellor]

Clang warns (or errors with CONFIG_WERROR=y) when performing arithmetic
with different enumerated types, which is usually a bug:


drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_dpia_bw.c:548:24:
 error: arithmetic between different enumeration types ('const enum 
dc_link_rate' and 'const enum dc_lane_count') [-Werror,-Wenum-enum-conversion]
  548 | link_cap->link_rate * link_cap->lane_count 
* LINK_RATE_REF_FREQ_IN_KHZ * 8;
  | ~~~ ^ 
1 error generated.

In this case, there is not a problem because the enumerated types are
basically treated as '#define' values. Add an explicit cast to an
integral type to silence the warning.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1976
Fixes: 5f3bce13266e ("drm/amd/display: Request usb4 bw for mst streams")
Signed-off-by: Nathan Chancellor 
---
 drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c 
b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c
index 4ef1a6a1d129..dd0d2b206462 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c
@@ -544,8 +544,9 @@ int link_dp_dpia_get_dp_overhead_in_dp_tunneling(struct 
dc_link *link)
 */
const struct dc_link_settings *link_cap =
dc_link_get_link_cap(link);
-   uint32_t link_bw_in_kbps =
-   link_cap->link_rate * link_cap->lane_count * 
LINK_RATE_REF_FREQ_IN_KHZ * 8;
+   uint32_t link_bw_in_kbps = (uint32_t)link_cap->link_rate *
+  (uint32_t)link_cap->lane_count *
+  LINK_RATE_REF_FREQ_IN_KHZ * 8;
link_mst_overhead = (link_bw_in_kbps / 64) + ((link_bw_in_kbps 
% 64) ? 1 : 0);
}
 

---
base-commit: 6e7a29f011ac03a765f53844f7c3f04cdd421715
change-id: 20240110-amdgpu-display-enum-enum-conversion-e2451adbf4a7

Best regards,
-- 
Nathan Chancellor 

RE: Documentation for RGB strip on RX 7900 XTX (Reference)

[Deucher, Alexander]

[Public]

> -Original Message-
> From: Alexander Koskovich 
> Sent: Tuesday, January 9, 2024 6:01 PM
> To: Deucher, Alexander 
> Cc: amd-gfx@lists.freedesktop.org
> Subject: RE: Documentation for RGB strip on RX 7900 XTX (Reference)
>
> I initially tried reaching out to the Coolermaster technical email, but I got 
> no
> response. Is there a better contact for something like this?
>

I'm not really sure.  That sounds like a good start.  It may work similarly to 
other CoolerMaster designs in openRGB.

Alex


>
>
> On Tuesday, January 9th, 2024 at 5:58 PM, Deucher, Alexander
>  wrote:
>
>
> >
> >
> > [Public]
> >
> > > -Original Message-
> > > From: amd-gfx amd-gfx-boun...@lists.freedesktop.org On Behalf Of
> > > Deucher, Alexander
> > > Sent: Tuesday, January 9, 2024 5:29 PM
> > > To: Alexander Koskovich akoskov...@protonmail.com
> > > Cc: amd-gfx@lists.freedesktop.org
> > > Subject: RE: Documentation for RGB strip on RX 7900 XTX (Reference)
> > >
> > > > -Original Message-
> > > > From: Alexander Koskovich akoskov...@protonmail.com
> > > > Sent: Tuesday, January 9, 2024 4:59 PM
> > > > To: Deucher, Alexander alexander.deuc...@amd.com
> > > > Cc: amd-gfx@lists.freedesktop.org
> > > > Subject: RE: Documentation for RGB strip on RX 7900 XTX
> > > > (Reference)
> > > >
> > > > Is the AIB/OEM for this board not AMD?
> > > > https://www.amd.com/en/products/graphics/amd-radeon-rx-7900xtx
> > >
> > > I'll double check (we usually don't produce reference boards with
> > > RGB), but my understanding is that if any of the boards available
> > > for sale on amd.com has RGB controls, the RGB control is provided by a
> third party vendor.
> >
> >
> >
> > CoolerMaster provides the RGB solution. See:
> > https://www.amd.com/en/support/graphics/amd-radeon-rx-7000-
> series/amd-
> > radeon-rx-7900-series/amd-radeon-rx-7900xtx
> >
> > Alex
> >
> > > Alex
> > >
> > > > On Tuesday, January 9th, 2024 at 4:53 PM, Deucher, Alexander
> > > > alexander.deuc...@amd.com wrote:
> > > >
> > > > > [AMD Official Use Only - General]
> > > > >
> > > > > > -Original Message-
> > > > > > From: Alexander Koskovich akoskov...@protonmail.com
> > > > > > Sent: Tuesday, January 9, 2024 3:27 PM
> > > > > > To: Deucher, Alexander alexander.deuc...@amd.com
> > > > > > Cc: amd-gfx@lists.freedesktop.org
> > > > > > Subject: RE: Documentation for RGB strip on RX 7900 XTX
> > > > > > (Reference)
> > > > > >
> > > > > > Doe AMD have documentation on the i2c data that gets sent
> > > > > > currently though? I was hoping to figure out what you need to
> > > > > > change in the command that gets sent to change stuff like
> > > > > > brightness, color (red, green, blue), rainbow, morse code, etc.
> > > > >
> > > > > It depends on the LED controller used by the AIB/OEM. The
> > > > > programming sequence is dependent on the LED controller.
> > > > >
> > > > > Alex
> > > > >
> > > > > > On Tuesday, January 9th, 2024 at 10:10 AM, Deucher, Alexander
> > > > > > alexander.deuc...@amd.com wrote:
> > > > > >
> > > > > > > [Public]
> > > > > > >
> > > > > > > > -Original Message-
> > > > > > > > From: Alexander Koskovich akoskov...@protonmail.com
> > > > > > > > Sent: Monday, January 8, 2024 7:22 PM
> > > > > > > > To: Deucher, Alexander alexander.deuc...@amd.com
> > > > > > > > Cc: amd-gfx@lists.freedesktop.org
> > > > > > > > Subject: RE: Documentation for RGB strip on RX 7900 XTX
> > > > > > > > (Reference)
> > > > > > > >
> > > > > > > > Currently the reference cooler from AMD does not have an
> > > > > > > > existing RGB controller for OpenRGB, that's why I was
> > > > > > > > looking for documentation on the I2C commands to send to
> > > > > > > > the second SMU, so I don't risk bricking my card by
> > > > > > > > sending wrong commands during development somehow.
> > > > > > > >
> > > > > > > > writeSetCMDWithData:
> > > >
> > > > **
> > > >
> > > > > > > > adli2c.iSize = sizeof(ADLI2C) adli2c.iAction =
> > > > > > > > ADL_DL_I2C_ACTIONWRITE adli2c.iAddress = 0xb4
> > > > > > > > adli2c.iSpeed = 100
> > > > > > > > 0 --
> > > > > > > > Dev 0: ADL_Display_WriteAndReadSMUI2C(0, ) = 0
> > > > > > > > adli2c.iDataSize =
> > > > > > > > 24 i2cData[0]~[24]
> > > > > > > > 40 51 2c 01 00 00 ff 00 ff ff ff cc 00 cc 00 00 00 ff ff
> > > > > > > > ff ff ff ff ff
> > > > > > > >
> > > > > > > > From the RGB app's logs this is an example of what the
> > > > > > > > official AMD application on Windows is sending when it
> > > > > > > > changes colors on the RGB strip.
> > > > > > > >
> > > > > > > > From this can it be assumed the AMD card is using the
> > > > > > > > latter method you mentioned with the second SMU I2C bus,
> > > > > > > > in which case no driver changes would be needed?
> > > > > > >
> > > > > > > IIRC, each AIB/OEM uses its own preferred RGB controller.
> > > > > > > The reference board just defines which 

Re: [PATCH v2] amd/amdkfd: Set correct svm range actual loc after spliting

[Philip Yang]

  


On 2024-01-10 11:30, Felix Kuehling
  wrote:


  
  On 2024-01-09 15:05, Philip Yang wrote:
  
  After svm range partial migrating to
system memory, unmap to cleanup the

corresponding dma_addr vram domain flag, otherwise the future
split will

get incorrect vram_pages and actual loc.


After range spliting, set new range and old range actual_loc:

new range actual_loc is 0 if new->vram_pages is 0.

old range actual_loc is 0 if old->vram_pages -
new->vram_pages == 0.


new range takes svm_bo ref only if vram_pages not equal to 0.


Signed-off-by: Philip Yang 

---

  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c |  3 ++

  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 35
+++-

  drivers/gpu/drm/amd/amdkfd/kfd_svm.h |  3 +-

  3 files changed, 27 insertions(+), 14 deletions(-)


diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c

index f856901055d3..e85bcda29db6 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c

@@ -839,6 +839,9 @@ int svm_migrate_vram_to_ram(struct svm_range
*prange, struct mm_struct *mm,

  prange->actual_loc = 0;

  svm_range_vram_node_free(prange);

  }

+

+    svm_range_dma_unmap(prange, start_mgr -
prange->start,

+    last_mgr - start_mgr + 1);

  
  
  If this is just for clearing the VRAM flags, then we should
  probably create another helper function for that. DMA unmapping
  system memory pages that didn't even move is not necessary here.
  
  
  Also, as Xiaogang pointed out, the migration may have missed some
  pages due to page locking race conditions. If you want this to
  give you accurate VRAM page counts, you should only clear the VRAM
  flags for pages that were actually migrated.
  

ok, understand the concern now, if failed to migrate page to
  system memory to recover CPU page fault, app will crash, but
  prefetch may fail to migrate page to system memory, will send new
  patch, to clear the prange->dma_addr[gpuidx] VRAM flags while
  migrating the range to ram.
Regards,
Philip


  
  Regards,
  
    Felix
  
  
  
    }

    return r < 0 ? r : 0;

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c

index cc24f30f88fb..2202bdcde057 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c

@@ -254,6 +254,10 @@ void svm_range_dma_unmap_dev(struct device
*dev, dma_addr_t *dma_addr,

  return;

    for (i = offset; i < offset + npages; i++) {

+    if (dma_addr[i] & SVM_RANGE_VRAM_DOMAIN) {

+    dma_addr[i] = 0;

+    continue;

+    }

  if (!svm_is_valid_dma_mapping_addr(dev, dma_addr[i]))

  continue;

  pr_debug_ratelimited("unmap 0x%llx\n", dma_addr[i]
>> PAGE_SHIFT);

@@ -262,7 +266,8 @@ void svm_range_dma_unmap_dev(struct device
*dev, dma_addr_t *dma_addr,

  }

  }

  -void svm_range_dma_unmap(struct svm_range *prange)

+void svm_range_dma_unmap(struct svm_range *prange, unsigned
long offset,

+ unsigned long npages)

  {

  struct kfd_process_device *pdd;

  dma_addr_t *dma_addr;

@@ -284,7 +289,7 @@ void svm_range_dma_unmap(struct svm_range
*prange)

  }

  dev = >dev->adev->pdev->dev;

  -    svm_range_dma_unmap_dev(dev, dma_addr, 0,
prange->npages);

+    svm_range_dma_unmap_dev(dev, dma_addr, offset, npages);

  }

  }

  @@ -299,7 +304,7 @@ static void svm_range_free(struct
svm_range *prange, bool do_unmap)

    svm_range_vram_node_free(prange);

  if (do_unmap)

-    svm_range_dma_unmap(prange);

+    svm_range_dma_unmap(prange, 0, 

RE: [PATCH] drm/amdgpu: drop exp hw support check for GC 9.4.3

[Deucher, Alexander]

[Public]

Ping!

> -Original Message-
> From: Deucher, Alexander 
> Sent: Tuesday, January 9, 2024 10:46 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: [PATCH] drm/amdgpu: drop exp hw support check for GC 9.4.3
>
> No longer needed.
>
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index b8fde08aec8e..f96811bbe40e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -1963,8 +1963,6 @@ static int
> amdgpu_discovery_set_gc_ip_blocks(struct amdgpu_device *adev)
>   amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
>   break;
>   case IP_VERSION(9, 4, 3):
> - if (!amdgpu_exp_hw_support)
> - return -EINVAL;
>   amdgpu_device_ip_block_add(adev, _v9_4_3_ip_block);
>   break;
>   case IP_VERSION(10, 1, 10):
> --
> 2.42.0

Re: [PATCH] Revert "drm/amdkfd: Relocate TBA/TMA to opposite side of VM hole"

[Marek Olšák]

It looks like this would cause failures even with regular 64-bit
allocations because the virtual address range allocator in libdrm asks
the kernel what ranges of addresses are free, and the kernel doesn't
exclude the KFD allocation from that.

Basically, no VM allocations can be done by the kernel outside the
ranges reserved for the kernel.

Marek

On Sat, Jan 6, 2024 at 1:48 AM Marek Olšák  wrote:
>
> The 32-bit address space means the high 32 bits are constant and 
> predetermined and it's definitely somewhere in the upper range of the address 
> space. If ROCm or KFD occupy that space, even accidentally, other UMDs that 
> use libdrm for VA allocation won't be able to start. The VA range allocator 
> is in libdrm.
>
> Marek
>
> On Fri, Jan 5, 2024, 15:20 Felix Kuehling  wrote:
>>
>> TBA/TMA were relocated to the upper half of the canonical address space.
>> I don't think that qualifies as 32-bit by definition. But maybe you're
>> using a different definition.
>>
>> That said, if Mesa manages its own virtual address space in user mode,
>> and KFD maps the TMA/TBA at an address that Mesa believes to be free, I
>> can see how that would lead to problems.
>>
>> That said, the fence refcount bug is another problem that may have been
>> exposed by the way that a crashing Mesa application shuts down.
>> Reverting Jay's patch certainly didn't fix that, but only hides the problem.
>>
>> Regards,
>>Felix
>>
>>
>> On 2024-01-04 13:29, Marek Olšák wrote:
>> > Hi,
>> >
>> > I have received information that the original commit makes all 32-bit
>> > userspace VA allocations fail, so UMDs like Mesa can't even initialize
>> > and they either crash or fail to load. If TBA/TMA was relocated to the
>> > 32-bit address range, it would explain why UMDs can't allocate
>> > anything in that range.
>> >
>> > Marek
>> >
>> > On Wed, Jan 3, 2024 at 2:50 PM Jay Cornwall  wrote:
>> >> On 1/3/2024 12:58, Felix Kuehling wrote:
>> >>
>> >>> A segfault in Mesa seems to be a different issue from what's mentioned
>> >>> in the commit message. I'd let Christian or Marek comment on
>> >>> compatibility with graphics UMDs. I'm not sure why this patch would
>> >>> affect them at all.
>> >> I was referencing this issue in OpenCL/OpenGL interop, which certainly 
>> >> looked related:
>> >>
>> >> [   91.769002] amdgpu :0a:00.0: amdgpu: bo 9bba4692 va 
>> >> 0x08-0x0801ff conflict with 0x08-0x080002
>> >> [   91.769141] ocltst[2781]: segfault at b2 ip 7f3fb90a7c39 sp 
>> >> 7ffd3c011ba0 error 4 in radeonsi_dri.so[7f3fb888e000+1196000] likely 
>> >> on CPU 15 (core 7, socket 0)
>> >>
>> >>> Looking at the logs in the tickets, it looks like a fence reference
>> >>> counting error. I don't see how Jay's patch could have caused that. I
>> >>> made another change in that code recently that could make a difference
>> >>> for this issue:
>> >>>
>> >>>  commit 8f08c5b24ced1be7eb49692e4816c1916233c79b
>> >>>  Author: Felix Kuehling 
>> >>>  Date:   Fri Oct 27 18:21:55 2023 -0400
>> >>>
>> >>>   drm/amdkfd: Run restore_workers on freezable WQs
>> >>>
>> >>>   Make restore workers freezable so we don't have to explicitly
>> >>>  flush them
>> >>>   in suspend and GPU reset code paths, and we don't accidentally
>> >>>  try to
>> >>>   restore BOs while the GPU is suspended. Not having to flush
>> >>>  restore_work
>> >>>   also helps avoid lock/fence dependencies in the GPU reset case
>> >>>  where we're
>> >>>   not allowed to wait for fences.
>> >>>
>> >>>   A side effect of this is, that we can now have multiple
>> >>>  concurrent threads
>> >>>   trying to signal the same eviction fence. Rework eviction fence
>> >>>  signaling
>> >>>   and replacement to account for that.
>> >>>
>> >>>   The GPU reset path can no longer rely on restore_process_worker
>> >>>  to resume
>> >>>   queues because evict/restore workers can run independently of
>> >>>  it. Instead
>> >>>   call a new restore_process_helper directly.
>> >>>
>> >>>   This is an RFC and request for testing.
>> >>>
>> >>>   v2:
>> >>>   - Reworked eviction fence signaling
>> >>>   - Introduced restore_process_helper
>> >>>
>> >>>   v3:
>> >>>   - Handle unsignaled eviction fences in restore_process_bos
>> >>>
>> >>>   Signed-off-by: Felix Kuehling 
>> >>>   Acked-by: Christian König 
>> >>>   Tested-by: Emily Deng 
>> >>>   Signed-off-by: Alex Deucher 
>> >>>
>> >>>
>> >>> FWIW, I built a plain 6.6 kernel, and was not able to reproduce the
>> >>> crash with some simple tests.
>> >>>
>> >>> Regards,
>> >>> Felix
>> >>>
>> >>>
>>  So I agree, let's revert it.
>> 
>>  Reviewed-by: Jay Cornwall 

Re: [PATCH 3/7] drm/amd/display: Add handling for new "active color format" property

[Andri Yngvason]

Hi Werner,

mið., 10. jan. 2024 kl. 13:14 skrifaði Werner Sembach 
:
>
> Hi,
>
> Am 10.01.24 um 14:09 schrieb Daniel Vetter:
> > On Wed, 10 Jan 2024 at 13:53, Andri Yngvason  wrote:
> >> mið., 10. jan. 2024 kl. 11:10 skrifaði Daniel Vetter :
> >>> On Tue, Jan 09, 2024 at 06:11:00PM +, Andri Yngvason wrote:
>  + /* Extract information from crtc to communicate it to userspace as 
>  connector properties */
>  + for_each_new_connector_in_state(state, connector, new_con_state, 
>  i) {
>  + struct drm_crtc *crtc = new_con_state->crtc;
>  + struct dc_stream_state *stream;
>  +
>  + if (crtc) {
>  + new_crtc_state = 
>  drm_atomic_get_new_crtc_state(state, crtc);
>  + dm_new_crtc_state = 
>  to_dm_crtc_state(new_crtc_state);
>  + stream = dm_new_crtc_state->stream;
>  +
>  + if (stream) {
>  + 
>  drm_connector_set_active_color_format_property(connector,
>  + 
>  convert_dc_pixel_encoding_into_drm_color_format(
>  + 
>  dm_new_crtc_state->stream->timing.pixel_encoding));
>  + }
>  + } else {
>  + 
>  drm_connector_set_active_color_format_property(connector, 0);
> >>> Just realized an even bigger reason why your current design doesn't work:
> >>> You don't have locking here.
> >>>
> >>> And you cannot grab the required lock, which is
> >>> drm_dev->mode_config.mutex, because that would result in deadlocks. So
> >>> this really needs to use the atomic state based design I've described.
> >>>
> >> Maybe we should just drop "actual color format" and instead fail the
> >> modeset if the "preferred color format" property cannot be satisfied?
> >> It seems like the simplest thing to do here, though it is perhaps less
> >> convenient for userspace. In that case, the "preferred color format"
> >> property should just be called "color format".
> > Yeah that's more in line with how other atomic properties work. This
> > way userspace can figure out what works with a TEST_ONLY commit too.
> > And for this to work you probably want to have an "automatic" setting
> > too.
> > -Sima
>
> The problem with TEST_ONLY probing is that color format settings are
> interdependent: 
> https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_966634
>
> So changing any other setting may require every color format to be TEST_ONLY
> probed again.
>

If we put a bit map containing the possible color formats into
drm_mode_mode_info (I'm thinking that it could go into flags), we'd be
able to eliminate a bunch of combinations early on. Do you think that
would make things more bearable?

I'm thinking, something like this:
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 128d09138ceb3..59980803cb89e 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -124,6 +124,13 @@ extern "C" {
 #define  DRM_MODE_FLAG_PIC_AR_256_135 \
(DRM_MODE_PICTURE_ASPECT_256_135<<19)

+/* Possible color formats (4 bits) */
+#define DRM_MODE_FLAG_COLOR_FORMAT_MASK (0x0f << 22)
+#define DRM_MODE_FLAG_COLOR_FORMAT_RGB (1 << 22)
+#define DRM_MODE_FLAG_COLOR_FORMAT_YCBCR444 (1 << 23)
+#define DRM_MODE_FLAG_COLOR_FORMAT_YCBCR422 (1 << 24)
+#define DRM_MODE_FLAG_COLOR_FORMAT_YCBCR420 (1 << 25)
+
 #define  DRM_MODE_FLAG_ALL (DRM_MODE_FLAG_PHSYNC | \
 DRM_MODE_FLAG_NHSYNC | \
 DRM_MODE_FLAG_PVSYNC | \
@@ -136,7 +143,8 @@ extern "C" {
 DRM_MODE_FLAG_HSKEW |  \
 DRM_MODE_FLAG_DBLCLK | \
 DRM_MODE_FLAG_CLKDIV2 |\
-DRM_MODE_FLAG_3D_MASK)
+DRM_MODE_FLAG_3D_MASK |\
+DRM_MODE_FLAG_COLOR_FORMAT_MASK)

 /* DPMS flags */
 /* bit compatible with the xorg definitions. */

Re: [PATCH v2] amd/amdkfd: Set correct svm range actual loc after spliting

[Felix Kuehling]

On 2024-01-09 15:05, Philip Yang wrote:

After svm range partial migrating to system memory, unmap to cleanup the
corresponding dma_addr vram domain flag, otherwise the future split will
get incorrect vram_pages and actual loc.

After range spliting, set new range and old range actual_loc:
new range actual_loc is 0 if new->vram_pages is 0.
old range actual_loc is 0 if old->vram_pages - new->vram_pages == 0.

new range takes svm_bo ref only if vram_pages not equal to 0.

Signed-off-by: Philip Yang 
---
  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c |  3 ++
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 35 +++-
  drivers/gpu/drm/amd/amdkfd/kfd_svm.h |  3 +-
  3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index f856901055d3..e85bcda29db6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -839,6 +839,9 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, 
struct mm_struct *mm,
prange->actual_loc = 0;
svm_range_vram_node_free(prange);
}
+
+   svm_range_dma_unmap(prange, start_mgr - prange->start,
+   last_mgr - start_mgr + 1);


If this is just for clearing the VRAM flags, then we should probably 
create another helper function for that. DMA unmapping system memory 
pages that didn't even move is not necessary here.


Also, as Xiaogang pointed out, the migration may have missed some pages 
due to page locking race conditions. If you want this to give you 
accurate VRAM page counts, you should only clear the VRAM flags for 
pages that were actually migrated.


Regards,
  Felix



}
  
  	return r < 0 ? r : 0;

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index cc24f30f88fb..2202bdcde057 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -254,6 +254,10 @@ void svm_range_dma_unmap_dev(struct device *dev, 
dma_addr_t *dma_addr,
return;
  
  	for (i = offset; i < offset + npages; i++) {

+   if (dma_addr[i] & SVM_RANGE_VRAM_DOMAIN) {
+   dma_addr[i] = 0;
+   continue;
+   }
if (!svm_is_valid_dma_mapping_addr(dev, dma_addr[i]))
continue;
pr_debug_ratelimited("unmap 0x%llx\n", dma_addr[i] >> 
PAGE_SHIFT);
@@ -262,7 +266,8 @@ void svm_range_dma_unmap_dev(struct device *dev, dma_addr_t 
*dma_addr,
}
  }
  
-void svm_range_dma_unmap(struct svm_range *prange)

+void svm_range_dma_unmap(struct svm_range *prange, unsigned long offset,
+unsigned long npages)
  {
struct kfd_process_device *pdd;
dma_addr_t *dma_addr;
@@ -284,7 +289,7 @@ void svm_range_dma_unmap(struct svm_range *prange)
}
dev = >dev->adev->pdev->dev;
  
-		svm_range_dma_unmap_dev(dev, dma_addr, 0, prange->npages);

+   svm_range_dma_unmap_dev(dev, dma_addr, offset, npages);
}
  }
  
@@ -299,7 +304,7 @@ static void svm_range_free(struct svm_range *prange, bool do_unmap)
  
  	svm_range_vram_node_free(prange);

if (do_unmap)
-   svm_range_dma_unmap(prange);
+   svm_range_dma_unmap(prange, 0, prange->npages);
  
  	if (do_unmap && !p->xnack_enabled) {

pr_debug("unreserve prange 0x%p size: 0x%llx\n", prange, size);
@@ -362,7 +367,6 @@ svm_range *svm_range_new(struct svm_range_list *svms, 
uint64_t start,
INIT_LIST_HEAD(>child_list);
atomic_set(>invalid, 0);
prange->validate_timestamp = 0;
-   prange->vram_pages = 0;
mutex_init(>migrate_mutex);
mutex_init(>lock);
  
@@ -980,9 +984,14 @@ svm_range_split_pages(struct svm_range *new, struct svm_range *old,

if (r)
return r;
}
-   if (old->actual_loc)
+   if (old->actual_loc && new->vram_pages) {
old->vram_pages -= new->vram_pages;
-
+   new->actual_loc = old->actual_loc;
+   if (!old->vram_pages)
+   old->actual_loc = 0;
+   }
+   pr_debug("new->vram_pages 0x%llx loc 0x%x old->vram_pages 0x%llx loc 
0x%x\n",
+new->vram_pages, new->actual_loc, old->vram_pages, 
old->actual_loc);
return 0;
  }
  
@@ -1002,13 +1011,14 @@ svm_range_split_nodes(struct svm_range *new, struct svm_range *old,

new->offset = old->offset + npages;
}
  
-	new->svm_bo = svm_range_bo_ref(old->svm_bo);

-   new->ttm_res = old->ttm_res;
-
-   spin_lock(>svm_bo->list_lock);
-   list_add(>svm_bo_list, >svm_bo->range_list);
-   spin_unlock(>svm_bo->list_lock);
+   if (new->vram_pages) {
+   new->svm_bo = svm_range_bo_ref(old->svm_bo);
+ 

Re: [PATCH] drm/amdkfd: Fix the shift-out-of-bounds warning

[Felix Kuehling]

On 2024-01-10 04:39, Ma Jun wrote:

There is following shift-out-of-bounds warning if ecode=0.
"shift exponent 4294967295 is too large for 64-bit type 'long long unsigned 
int'"

Signed-off-by: Ma Jun 
---
  include/uapi/linux/kfd_ioctl.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index 2aa88afe305b..129325b02a91 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -1004,7 +1004,7 @@ enum kfd_dbg_trap_exception_code {
  };
  
  /* Mask generated by ecode in kfd_dbg_trap_exception_code */

-#define KFD_EC_MASK(ecode) (1ULL << (ecode - 1))
+#define KFD_EC_MASK(ecode) (BIT(ecode) - 1)


This is not the same thing. We want a bit mask with one bit set. And 
ecode=1 should set bit 0. ecode=0 is not a valid code and doesn't have a 
valid mask. You could use BIT((ecode) - 1), but I think that would give 
you the same warning for ecode=0. I also don't see BIT defined anywhere 
under include/uapi, so I think using this in the API header would break 
the build in user mode.


Where are you seeing the warning about the bad shift exponent? Looks 
like someone is using the KFD_EC_MASK macro incorrectly. Or if there is 
a legitimate use of it with ecode=0, then the correct fix would be


#define KFD_EC_MASK(ecode)  ((ecode) ? 1ULL << (ecode - 1) : 0ULL)

Regards,
  Felix


  
  /* Masks for exception code type checks below */

  #define KFD_EC_MASK_QUEUE (KFD_EC_MASK(EC_QUEUE_WAVE_ABORT) | \

[PATCH v2] drm/amdkfd: Fix variable dereferenced before NULL check in 'kfd_dbg_trap_device_snapshot()'

[Srinivasan Shanmugam]

Fixes the below:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_debug.c:1024 
kfd_dbg_trap_device_snapshot() warn: variable dereferenced before check 
'entry_size' (see line 1021)

Cc: Felix Kuehling 
Cc: Christian König 
Cc: Alex Deucher 
Signed-off-by: Srinivasan Shanmugam 
---
v2:
 - Changed from u32 to unit32_t for consistency (Felix) 

 drivers/gpu/drm/amd/amdkfd/kfd_debug.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
index 9ec750666382..d889e3545120 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
@@ -1018,12 +1018,14 @@ int kfd_dbg_trap_device_snapshot(struct kfd_process 
*target,
uint32_t *entry_size)
 {
struct kfd_dbg_device_info_entry device_info;
-   uint32_t tmp_entry_size = *entry_size, tmp_num_devices;
+   uint32_t tmp_entry_size, tmp_num_devices;
int i, r = 0;
 
if (!(target && user_info && number_of_device_infos && entry_size))
return -EINVAL;
 
+   tmp_entry_size = *entry_size;
+
tmp_num_devices = min_t(size_t, *number_of_device_infos, 
target->n_pdds);
*number_of_device_infos = target->n_pdds;
*entry_size = min_t(size_t, *entry_size, sizeof(device_info));
-- 
2.34.1

[PATCH] drm/amd/display: Fix late derefrence 'dsc' check in 'link_set_dsc_pps_packet()'

[Srinivasan Shanmugam]

In link_set_dsc_pps_packet(), 'struct display_stream_compressor *dsc'
was dereferenced in a DC_LOGGER_INIT(dsc->ctx->logger); before the 'dsc'
NULL pointer check.

Fixes the below:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_dpms.c:905 
link_set_dsc_pps_packet() warn: variable dereferenced before check 'dsc' (see 
line 903)

Cc: sta...@vger.kernel.org
Cc: Aurabindo Pillai 
Cc: Rodrigo Siqueira 
Cc: Hamza Mahfooz 
Cc: Wenjing Liu 
Cc: Qingqing Zhuo 
Signed-off-by: Srinivasan Shanmugam 
---
 drivers/gpu/drm/amd/display/dc/link/link_dpms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c 
b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
index 3de148004c06..562eb79bfbc8 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
@@ -900,11 +900,12 @@ bool link_set_dsc_pps_packet(struct pipe_ctx *pipe_ctx, 
bool enable, bool immedi
 {
struct display_stream_compressor *dsc = pipe_ctx->stream_res.dsc;
struct dc_stream_state *stream = pipe_ctx->stream;
-   DC_LOGGER_INIT(dsc->ctx->logger);
 
if (!pipe_ctx->stream->timing.flags.DSC || !dsc)
return false;
 
+   DC_LOGGER_INIT(dsc->ctx->logger);
+
if (enable) {
struct dsc_config dsc_cfg;
uint8_t dsc_packed_pps[128];
-- 
2.34.1

Re: [PATCH] drm/amdkfd: Fix 'node' NULL check in 'svm_range_get_range_boundaries()'

[Felix Kuehling]

On 2024-01-10 03:26, Srinivasan Shanmugam wrote:

Range interval [start, last] is ordered by rb_tree, rb_prev, rb_next
return value still needs NULL check, thus modified from "node" to "rb_node".

Fixes the below:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c:2691 
svm_range_get_range_boundaries() warn: can 'node' even be NULL?

Suggested-by: Philip Yang 
Cc: Felix Kuehling 
Cc: Christian König 
Cc: Alex Deucher 
Signed-off-by: Srinivasan Shanmugam 


Reviewed-by: Felix Kuehling 



---
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 3b78e48832e9..6aa032731ddc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2656,6 +2656,7 @@ svm_range_get_range_boundaries(struct kfd_process *p, 
int64_t addr,
  {
struct vm_area_struct *vma;
struct interval_tree_node *node;
+   struct rb_node *rb_node;
unsigned long start_limit, end_limit;
  
  	vma = vma_lookup(p->mm, addr << PAGE_SHIFT);

@@ -2678,16 +2679,15 @@ svm_range_get_range_boundaries(struct kfd_process *p, 
int64_t addr,
if (node) {
end_limit = min(end_limit, node->start);
/* Last range that ends before the fault address */
-   node = container_of(rb_prev(>rb),
-   struct interval_tree_node, rb);
+   rb_node = rb_prev(>rb);
} else {
/* Last range must end before addr because
 * there was no range after addr
 */
-   node = container_of(rb_last(>svms.objects.rb_root),
-   struct interval_tree_node, rb);
+   rb_node = rb_last(>svms.objects.rb_root);
}
-   if (node) {
+   if (rb_node) {
+   node = container_of(rb_node, struct interval_tree_node, rb);
if (node->last >= addr) {
WARN(1, "Overlap with prev node and page fault addr\n");
return -EFAULT;

Re: [PATCH v2] amd/amdkfd: Set correct svm range actual loc after spliting

[Philip Yang]

  


On 2024-01-09 17:29, Chen, Xiaogang
  wrote:


  
  On 1/9/2024 2:05 PM, Philip Yang wrote:
  
  After svm range partial migrating to
system memory, unmap to cleanup the

corresponding dma_addr vram domain flag, otherwise the future
split will

get incorrect vram_pages and actual loc.


After range spliting, set new range and old range actual_loc:

new range actual_loc is 0 if new->vram_pages is 0.

old range actual_loc is 0 if old->vram_pages -
new->vram_pages == 0.


new range takes svm_bo ref only if vram_pages not equal to 0.


Signed-off-by: Philip Yang 

---

  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c |  3 ++

  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 35
+++-

  drivers/gpu/drm/amd/amdkfd/kfd_svm.h |  3 +-

  3 files changed, 27 insertions(+), 14 deletions(-)


diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c

index f856901055d3..e85bcda29db6 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c

@@ -839,6 +839,9 @@ int svm_migrate_vram_to_ram(struct svm_range
*prange, struct mm_struct *mm,

  prange->actual_loc = 0;

  svm_range_vram_node_free(prange);

  }

+

+    svm_range_dma_unmap(prange, start_mgr -
prange->start,

+    last_mgr - start_mgr + 1);

  
  
  when come here we know some pages got migrated to sys ram, in
  theory we do not know if all pages got migrated.
  svm_range_dma_unmap does dma_unmap for all pages from  start_mgr -
  prange->start to  last_mgr - start_mgr + 1.
  
  
  If there are pages not migrated due to some reason(though it is
  rare) we still need keep its dma_addr, I think only hmm can tell
  that.
  

For system page dma unmap_page and set dma_addr=0 after migration
  is fine because before updating GPU mapping,
  svm_range_validate_and_map calls svm_range_dma_map to update
  dma_addr for system pages.

  
    }

    return r < 0 ? r : 0;

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c

index cc24f30f88fb..2202bdcde057 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c

@@ -254,6 +254,10 @@ void svm_range_dma_unmap_dev(struct device
*dev, dma_addr_t *dma_addr,

  return;

    for (i = offset; i < offset + npages; i++) {

+    if (dma_addr[i] & SVM_RANGE_VRAM_DOMAIN) {

+    dma_addr[i] = 0;

+    continue;

+    }

  
  same as above here set dma_addr[i]=0 unconditionally without
  knowing if the page is indeed in sys ram.
  

dma_addr[i] & SVM_RANGE_VRAM_DOMAIN is for device page,
  system page will still call dma_unmap_page.




    if
(!svm_is_valid_dma_mapping_addr(dev, dma_addr[i]))

  continue;

  pr_debug_ratelimited("unmap 0x%llx\n", dma_addr[i]
>> PAGE_SHIFT);

@@ -262,7 +266,8 @@ void svm_range_dma_unmap_dev(struct device
*dev, dma_addr_t *dma_addr,

  }

  }

  -void svm_range_dma_unmap(struct svm_range *prange)

+void svm_range_dma_unmap(struct svm_range *prange, unsigned
long offset,

+ unsigned long npages)

  {

  struct kfd_process_device *pdd;

  dma_addr_t *dma_addr;

@@ -284,7 +289,7 @@ void svm_range_dma_unmap(struct svm_range
*prange)

  }

  dev = >dev->adev->pdev->dev;

  -    svm_range_dma_unmap_dev(dev, dma_addr, 0,
prange->npages);

+    svm_range_dma_unmap_dev(dev, dma_addr, offset, npages);

  }

  }

  @@ -299,7 +304,7 @@ static void svm_range_free(struct
svm_range *prange, bool do_unmap)

    svm_range_vram_node_free(prange);

  if (do_unmap)

-    svm_range_dma_unmap(prange);

+    

Re: [PATCH v2 10/14] riscv: Add support for kernel-mode FPU

[Palmer Dabbelt]

On Wed, 27 Dec 2023 17:42:00 PST (-0800), samuel.holl...@sifive.com wrote:

This is motivated by the amdgpu DRM driver, which needs floating-point
code to support recent hardware. That code is not performance-critical,
so only provide a minimal non-preemptible implementation for now.

Signed-off-by: Samuel Holland 
---

Changes in v2:
 - Remove RISC-V architecture-specific preprocessor check

 arch/riscv/Kconfig  |  1 +
 arch/riscv/Makefile |  3 +++
 arch/riscv/include/asm/fpu.h| 16 
 arch/riscv/kernel/Makefile  |  1 +
 arch/riscv/kernel/kernel_mode_fpu.c | 28 
 5 files changed, 49 insertions(+)
 create mode 100644 arch/riscv/include/asm/fpu.h
 create mode 100644 arch/riscv/kernel/kernel_mode_fpu.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 24c1799e2ec4..4d4d1d64ce34 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -27,6 +27,7 @@ config RISCV
select ARCH_HAS_GCOV_PROFILE_ALL
select ARCH_HAS_GIGANTIC_PAGE
select ARCH_HAS_KCOV
+   select ARCH_HAS_KERNEL_FPU_SUPPORT if FPU
select ARCH_HAS_MMIOWB
select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
select ARCH_HAS_PMEM_API
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index a74be78678eb..2e719c369210 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -81,6 +81,9 @@ KBUILD_CFLAGS += -march=$(shell echo $(riscv-march-y) | sed 
-E 's/(rv32ima|rv64i

 KBUILD_AFLAGS += -march=$(riscv-march-y)

+# For C code built with floating-point support, exclude V but keep F and D.
+CC_FLAGS_FPU  := -march=$(shell echo $(riscv-march-y) | sed -E 
's/(rv32ima|rv64ima)([^v_]*)v?/\1\2/')
+
 KBUILD_CFLAGS += -mno-save-restore
 KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)

diff --git a/arch/riscv/include/asm/fpu.h b/arch/riscv/include/asm/fpu.h
new file mode 100644
index ..91c04c244e12
--- /dev/null
+++ b/arch/riscv/include/asm/fpu.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023 SiFive
+ */
+
+#ifndef _ASM_RISCV_FPU_H
+#define _ASM_RISCV_FPU_H
+
+#include 
+
+#define kernel_fpu_available() has_fpu()
+
+void kernel_fpu_begin(void);
+void kernel_fpu_end(void);
+
+#endif /* ! _ASM_RISCV_FPU_H */
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index fee22a3d1b53..662c483e338d 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_MMU) += vdso.o vdso/

 obj-$(CONFIG_RISCV_MISALIGNED) += traps_misaligned.o
 obj-$(CONFIG_FPU)  += fpu.o
+obj-$(CONFIG_FPU)  += kernel_mode_fpu.o
 obj-$(CONFIG_RISCV_ISA_V)  += vector.o
 obj-$(CONFIG_SMP)  += smpboot.o
 obj-$(CONFIG_SMP)  += smp.o
diff --git a/arch/riscv/kernel/kernel_mode_fpu.c 
b/arch/riscv/kernel/kernel_mode_fpu.c
new file mode 100644
index ..0ac8348876c4
--- /dev/null
+++ b/arch/riscv/kernel/kernel_mode_fpu.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 SiFive
+ */
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+void kernel_fpu_begin(void)
+{
+   preempt_disable();
+   fstate_save(current, task_pt_regs(current));
+   csr_set(CSR_SSTATUS, SR_FS);
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_begin);
+
+void kernel_fpu_end(void)
+{
+   csr_clear(CSR_SSTATUS, SR_FS);
+   fstate_restore(current, task_pt_regs(current));
+   preempt_enable();
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_end);


Reviewed-by: Palmer Dabbelt 
Acked-by: Palmer Dabbelt 

assuming you want to keep these together -- it touches a lot of stuff, 
so LMK if you want me to pick something up.


Thanks!

Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace

[Andri Yngvason]

Hi,

mið., 10. jan. 2024 kl. 09:27 skrifaði Maxime Ripard :
> On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote:
> > From: Werner Sembach 
> >
> > Add a new general drm property "preferred color format" which can be used
> > by userspace to tell the graphic drivers to which color format to use.
> >
> > Possible options are:
> > - auto (default/current behaviour)
> > - rgb
> > - ycbcr444
> > - ycbcr422 (not supported by both amdgpu and i915)
> > - ycbcr420
> >
> > In theory the auto option should choose the best available option for the
> > current setup, but because of bad internal conversion some monitors look
> > better with rgb and some with ycbcr444.
>
> I looked at the patch and I couldn't find what is supposed to happen if
> you set it to something else than auto, and the driver can't match that.
> Are we supposed to fallback to the "auto" behaviour, or are we suppose
> to reject the mode entirely?
>
> The combination with the active output format property suggests the
> former, but we should document it explicitly.

It is also my understanding that it should fall back to the "auto"
behaviour. I will add this to the documentation.

Thanks,
Andri

Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace

[Andri Yngvason]

mið., 10. jan. 2024 kl. 13:09 skrifaði Werner Sembach 
:
>
> Hi,
>
> Am 10.01.24 um 11:11 schrieb Andri Yngvason:
> > Hi,
> >
> > mið., 10. jan. 2024 kl. 09:27 skrifaði Maxime Ripard :
> >> On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote:
> >>> From: Werner Sembach 
> >>>
> >>> Add a new general drm property "preferred color format" which can be used
> >>> by userspace to tell the graphic drivers to which color format to use.
> >>>
> >>> Possible options are:
> >>>  - auto (default/current behaviour)
> >>>  - rgb
> >>>  - ycbcr444
> >>>  - ycbcr422 (not supported by both amdgpu and i915)
> >>>  - ycbcr420
> >>>
> >>> In theory the auto option should choose the best available option for the
> >>> current setup, but because of bad internal conversion some monitors look
> >>> better with rgb and some with ycbcr444.
> >> I looked at the patch and I couldn't find what is supposed to happen if
> >> you set it to something else than auto, and the driver can't match that.
> >> Are we supposed to fallback to the "auto" behaviour, or are we suppose
> >> to reject the mode entirely?
> >>
> >> The combination with the active output format property suggests the
> >> former, but we should document it explicitly.
> > It is also my understanding that it should fall back to the "auto"
> > behaviour. I will add this to the documentation.
>
> Yes, that was the intention, and then userspace can check, but it wasn't well
> received: https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_964530
>
> Actually a lot of the thoughts that went into the original patch set can be
> found in that topic.
>
> There was another iteration of the patch set that I never finished and sent to
> the LKML because I got discouraged by this:
> https://lore.kernel.org/dri-devel/20210623102923.70877c1a@eldfell/

Well, I've implemented this for sway and wlroots now and Simon has
reacted positively, so this does appear likely to end up as a feature
in wlroots based compositors.

>
> I can try to dig it up, but it is completely untested and I don't think I 
> still
> have the respective TODO list anymore, so I don't know if it is a better or
> worst starting point than the last iteration I sent to the LKML.
>

You can send the patches to me if you want and I can see if they're
useful. I'm really only interested in the color format part though.
Alternatively, you can continue your work and post it to LKML and I
can focus on the userspace side and testing. By the way, I have an
HDMI analyzer that can tell me the actual color format.

Thanks,
Andri

Re: [PATCH 3/7] drm/amd/display: Add handling for new "active color format" property

[Andri Yngvason]

mið., 10. jan. 2024 kl. 11:10 skrifaði Daniel Vetter :
>
> On Tue, Jan 09, 2024 at 06:11:00PM +, Andri Yngvason wrote:
> > + /* Extract information from crtc to communicate it to userspace as 
> > connector properties */
> > + for_each_new_connector_in_state(state, connector, new_con_state, i) {
> > + struct drm_crtc *crtc = new_con_state->crtc;
> > + struct dc_stream_state *stream;
> > +
> > + if (crtc) {
> > + new_crtc_state = drm_atomic_get_new_crtc_state(state, 
> > crtc);
> > + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> > + stream = dm_new_crtc_state->stream;
> > +
> > + if (stream) {
> > + 
> > drm_connector_set_active_color_format_property(connector,
> > + 
> > convert_dc_pixel_encoding_into_drm_color_format(
> > + 
> > dm_new_crtc_state->stream->timing.pixel_encoding));
> > + }
> > + } else {
> > + 
> > drm_connector_set_active_color_format_property(connector, 0);
>
> Just realized an even bigger reason why your current design doesn't work:
> You don't have locking here.
>
> And you cannot grab the required lock, which is
> drm_dev->mode_config.mutex, because that would result in deadlocks. So
> this really needs to use the atomic state based design I've described.
>

Maybe we should just drop "actual color format" and instead fail the
modeset if the "preferred color format" property cannot be satisfied?
It seems like the simplest thing to do here, though it is perhaps less
convenient for userspace. In that case, the "preferred color format"
property should just be called "color format".

Thanks,
Andri

Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace

[Werner Sembach]

Hi,

Am 10.01.24 um 14:42 schrieb Andri Yngvason:

mið., 10. jan. 2024 kl. 13:09 skrifaði Werner Sembach:

Hi,

Am 10.01.24 um 11:11 schrieb Andri Yngvason:

Hi,

mið., 10. jan. 2024 kl. 09:27 skrifaði Maxime Ripard:

On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote:

From: Werner Sembach

Add a new general drm property "preferred color format" which can be used
by userspace to tell the graphic drivers to which color format to use.

Possible options are:
  - auto (default/current behaviour)
  - rgb
  - ycbcr444
  - ycbcr422 (not supported by both amdgpu and i915)
  - ycbcr420

In theory the auto option should choose the best available option for the
current setup, but because of bad internal conversion some monitors look
better with rgb and some with ycbcr444.

I looked at the patch and I couldn't find what is supposed to happen if
you set it to something else than auto, and the driver can't match that.
Are we supposed to fallback to the "auto" behaviour, or are we suppose
to reject the mode entirely?

The combination with the active output format property suggests the
former, but we should document it explicitly.

It is also my understanding that it should fall back to the "auto"
behaviour. I will add this to the documentation.

Yes, that was the intention, and then userspace can check, but it wasn't well
received:https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_964530

Actually a lot of the thoughts that went into the original patch set can be
found in that topic.

There was another iteration of the patch set that I never finished and sent to
the LKML because I got discouraged by this:
https://lore.kernel.org/dri-devel/20210623102923.70877c1a@eldfell/

Well, I've implemented this for sway and wlroots now and Simon has
reacted positively, so this does appear likely to end up as a feature
in wlroots based compositors.


I can try to dig it up, but it is completely untested and I don't think I still
have the respective TODO list anymore, so I don't know if it is a better or
worst starting point than the last iteration I sent to the LKML.


You can send the patches to me if you want and I can see if they're
useful. I'm really only interested in the color format part though.
Alternatively, you can continue your work and post it to LKML and I
can focus on the userspace side and testing. By the way, I have an
HDMI analyzer that can tell me the actual color format.


Searched for what I still had in my private repo, see attachments, filename is 
the branch name I used and like I said: I don't know which state these branches 
are in.


The hacking_ branch was based on 25fe90f43fa312213b653dc1f12fd2d80f855883 from 
linux-next and the rejected_ one on 132b189b72a94328f17fd70321bfe63e5b4208e9 
from drm-tip.


And the rejected_ one is 2 weeks newer.

To pick it up again I would first need to allocate some time for it, ... which 
could take some time.


With a HDMI analyzer at hand you are better equipped then me already. I was 
working with printf statements, Monitor OSD's and test patterns like 
https://media.extron.com/public/technology/landing/vector4k/img/scalfe-444Chroma.png 
and http://www.lagom.nl/lcd-test/ while being red blind xD.




Thanks,
Andri

hacking_drm_color_management_no_immutable_flag.tar.gz
Description: application/gzip


rejected_drm_color_management.tar.gz
Description: application/gzip

[PATCH] drm/amdgpu: revert "Adjust removal control flow for smu v13_0_2"

[Christian König]

Calling amdgpu_device_ip_resume_phase1() during shutdown leaves the
HW in an active state and is an unbalanced use of the IP callbacks.

Using the IP callbacks like this can lead to memory leaks, double
free and imbalanced reference counters.

Leaving the HW in an active state can lead to DMA accesses to memory now
freed by the driver.

Both is a complete no-go for driver unload so completely revert the
workaround for now.

This reverts commit f5c7e7797060255dbc8160734ccc5ad6183c5e04.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 32 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 32 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  1 -
 4 files changed, 1 insertion(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a39c9fea55c4..313316009039 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5232,7 +5232,6 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
struct amdgpu_device *tmp_adev = NULL;
bool need_full_reset, skip_hw_reset, vram_lost = false;
int r = 0;
-   bool gpu_reset_for_dev_remove = 0;
 
/* Try reset handler method first */
tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
@@ -5252,10 +5251,6 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
test_bit(AMDGPU_NEED_FULL_RESET, _context->flags);
skip_hw_reset = test_bit(AMDGPU_SKIP_HW_RESET, _context->flags);
 
-   gpu_reset_for_dev_remove =
-   test_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, _context->flags) 
&&
-   test_bit(AMDGPU_NEED_FULL_RESET, _context->flags);
-
/*
 * ASIC reset has to be done on all XGMI hive nodes ASAP
 * to allow proper links negotiation in FW (within 1 sec)
@@ -5298,18 +5293,6 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
amdgpu_ras_intr_cleared();
}
 
-   /* Since the mode1 reset affects base ip blocks, the
-* phase1 ip blocks need to be resumed. Otherwise there
-* will be a BIOS signature error and the psp bootloader
-* can't load kdb on the next amdgpu install.
-*/
-   if (gpu_reset_for_dev_remove) {
-   list_for_each_entry(tmp_adev, device_list_handle, reset_list)
-   amdgpu_device_ip_resume_phase1(tmp_adev);
-
-   goto end;
-   }
-
list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
if (need_full_reset) {
/* post card */
@@ -5543,11 +5526,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
int i, r = 0;
bool need_emergency_restart = false;
bool audio_suspended = false;
-   bool gpu_reset_for_dev_remove = false;
-
-   gpu_reset_for_dev_remove =
-   test_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, 
_context->flags) &&
-   test_bit(AMDGPU_NEED_FULL_RESET, 
_context->flags);
 
/*
 * Special case: RAS triggered and full reset isn't supported
@@ -5585,7 +5563,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
if (!amdgpu_sriov_vf(adev) && (adev->gmc.xgmi.num_physical_nodes > 1)) {
list_for_each_entry(tmp_adev, >device_list, 
gmc.xgmi.head) {
list_add_tail(_adev->reset_list, _list);
-   if (gpu_reset_for_dev_remove && adev->shutdown)
+   if (adev->shutdown)
tmp_adev->shutdown = true;
}
if (!list_is_first(>reset_list, _list))
@@ -5670,10 +5648,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 
 retry: /* Rest of adevs pre asic reset from XGMI hive. */
list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
-   if (gpu_reset_for_dev_remove) {
-   /* Workaroud for ASICs need to disable SMC first */
-   amdgpu_device_smu_fini_early(tmp_adev);
-   }
r = amdgpu_device_pre_asic_reset(tmp_adev, reset_context);
/*TODO Should we stop ?*/
if (r) {
@@ -5705,9 +5679,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
r = amdgpu_do_asic_reset(device_list_handle, reset_context);
if (r && r == -EAGAIN)
goto retry;
-
-   if (!r && gpu_reset_for_dev_remove)
-   goto recover_end;
}
 
 skip_hw_reset:
@@ -5763,7 +5734,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
amdgpu_ras_set_error_query_ready(tmp_adev, true);
}
 
-recover_end:
tmp_adev = 

Re: [PATCH 2/3] arch and include: Update LLVM Phabricator links

[Conor Dooley]

On Tue, Jan 09, 2024 at 03:16:30PM -0700, Nathan Chancellor wrote:
> reviews.llvm.org was LLVM's Phabricator instances for code review. It
> has been abandoned in favor of GitHub pull requests. While the majority
> of links in the kernel sources still work because of the work Fangrui
> has done turning the dynamic Phabricator instance into a static archive,
> there are some issues with that work, so preemptively convert all the
> links in the kernel sources to point to the commit on GitHub.
> 
> Most of the commits have the corresponding differential review link in
> the commit message itself so there should not be any loss of fidelity in
> the relevant information.
> 
> Link: https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172
> Signed-off-by: Nathan Chancellor 
> ---

>  arch/riscv/Kconfig  | 2 +-
>  arch/riscv/include/asm/ftrace.h | 2 +-

Reviewed-by: Conor Dooley 

Cheers,
Conor.


signature.asc
Description: PGP signature

Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace

[Werner Sembach]

Hi,

Am 09.01.24 um 19:10 schrieb Andri Yngvason:

From: Werner Sembach 

Add a new general drm property "active color format" which can be used by
graphic drivers to report the used color format back to userspace.

There was no way to check which color format got actually used on a given
monitor. To surely predict this, one must know the exact capabilities of
the monitor, the GPU, and the connection used and what the default
behaviour of the used driver is (e.g. amdgpu prefers YCbCr 4:4:4 while i915
prefers RGB). This property helps eliminating the guessing on this point.

In the future, automatic color calibration for screens might also depend on
this information being available.

Signed-off-by: Werner Sembach 
Signed-off-by: Andri Yngvason 
Tested-by: Andri Yngvason 


One suggestion from back then was, instead picking out singular properties like 
"active color format", to just expose whatever the last HDMI or DP metadata 
block(s)/frame(s) that was sent over the display wire was to userspace and 
accompanying it with a parsing script.


Question: Does the driver really actually know what the GPU is ultimatively 
sending out the wire, or is that decided by a final firmware blob we have no 
info about?


Greetings

Werner


---
  drivers/gpu/drm/drm_connector.c | 63 +
  include/drm/drm_connector.h | 10 ++
  2 files changed, 73 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index c3725086f4132..30d62e505d188 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1061,6 +1061,14 @@ static const struct drm_prop_enum_list 
drm_dp_subconnector_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Native,  "Native"}, /* DP */
  };
  
+static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {

+   { 0, "not applicable" },
+   { DRM_COLOR_FORMAT_RGB444, "rgb" },
+   { DRM_COLOR_FORMAT_YCBCR444, "ycbcr444" },
+   { DRM_COLOR_FORMAT_YCBCR422, "ycbcr422" },
+   { DRM_COLOR_FORMAT_YCBCR420, "ycbcr420" },
+};
+
  DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
 drm_dp_subconnector_enum_list)
  
@@ -1390,6 +1398,15 @@ static const u32 dp_colorspaces =

   *drm_connector_attach_max_bpc_property() to create and attach the
   *property to the connector during initialization.
   *
+ * active color format:
+ * This read-only property tells userspace the color format 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. Drivers shall use
+ * drm_connector_attach_active_color_format_property() to install this
+ * property. Possible values are "not applicable", "rgb", "ycbcr444",
+ * "ycbcr422", and "ycbcr420".
+ *
   * Connectors also have one standardized atomic property:
   *
   * CRTC_ID:
@@ -2451,6 +2468,52 @@ int drm_connector_attach_max_bpc_property(struct 
drm_connector *connector,
  }
  EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
  
+/**

+ * drm_connector_attach_active_color_format_property - attach "active color 
format" property
+ * @connector: connector to attach active color format property on.
+ *
+ * This is used to check the applied color format on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_color_format_property(struct drm_connector 
*connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->active_color_format_property) {
+   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE, 
"active color format",
+   
drm_active_color_format_enum_list,
+   
ARRAY_SIZE(drm_active_color_format_enum_list));
+   if (!prop)
+   return -ENOMEM;
+
+   connector->active_color_format_property = prop;
+   }
+
+   drm_object_attach_property(>base, prop, 0);
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_color_format_property);
+
+/**
+ * drm_connector_set_active_color_format_property - sets the active color 
format property for a
+ * connector
+ * @connector: drm connector
+ * @active_color_format: color format for the connector currently active "on the 
cable"
+ *
+ * Should be used by atomic drivers to update the active color format over a 
connector.
+ */
+void drm_connector_set_active_color_format_property(struct drm_connector 
*connector,
+   u32 active_color_format)
+{
+   drm_object_property_set_value(>base, 
connector->active_color_format_property,
+ active_color_format);
+}
+EXPORT_SYMBOL(drm_connector_set_active_color_format_property);
+
  /**
   * 

Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace

[Daniel Stone]

Hi,

On Wed, 10 Jan 2024 at 10:44, Daniel Vetter  wrote:
> On Tue, Jan 09, 2024 at 11:12:11PM +, Andri Yngvason wrote:
> > ţri., 9. jan. 2024 kl. 22:32 skrifađi Daniel Stone :
> > > How does userspace determine what's happened without polling? Will it
> > > only change after an `ALLOW_MODESET` commit, and be guaranteed to be
> > > updated after the commit has completed and the event being sent?
> > > Should it send a HOTPLUG event? Other?
> > >
> >
> > Userspace does not determine what's happened without polling. The purpose
> > of this property is not for programmatic verification that the preferred
> > property was applied. It is my understanding that it's mostly intended for
> > debugging purposes. It should only change as a consequence of modesetting,
> > although I didn't actually look into what happens if you set the "preferred
> > color format" outside of a modeset.
>
> This feels a bit irky to me, since we don't have any synchronization and
> it kinda breaks how userspace gets to know about stuff.
>
> For context the current immutable properties are all stuff that's derived
> from the sink (like edid, or things like that). Userspace is guaranteed to
> get a hotplug event (minus driver bugs as usual) if any of these change,
> and we've added infrastructure so that the hotplug event even contains the
> specific property so that userspace can avoid re-read (which can cause
> some costly re-probing) them all.

Right.

> [...]
>
> This thing here works entirely differently, and I think we need somewhat
> new semantics for this:
>
> - I agree it should be read-only for userspace, so immutable sounds right.
>
> - But I also agree with Daniel Stone that this should be tied more
>   directly to the modeset state.
>
> So I think the better approach would be to put the output type into
> drm_connector_state, require that drivers compute it in their
> ->atomic_check code (which in the future would allow us to report it out
> for TEST_ONLY commits too), and so guarantee that the value is updated
> right after the kms ioctl returns (and not somewhen later for non-blocking
> commits).

That's exactly the point. Whether userspace gets an explicit
notification or it has to 'know' when to read isn't much of an issue -
just as long as it's well defined. I think the suggestion of 'do it in
atomic_check and then it's guaranteed to be readable when the commit
completes' is a good one.

I do still have some reservations - for instance, why do we have the
fallback to auto when userspace has explicitly requested a certain
type? - but they may have been covered previously.

Cheers,
Daniel

Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace

[Werner Sembach]

Hi,

Am 10.01.24 um 11:11 schrieb Andri Yngvason:

Hi,

mið., 10. jan. 2024 kl. 09:27 skrifaði Maxime Ripard :

On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote:

From: Werner Sembach 

Add a new general drm property "preferred color format" which can be used
by userspace to tell the graphic drivers to which color format to use.

Possible options are:
 - auto (default/current behaviour)
 - rgb
 - ycbcr444
 - ycbcr422 (not supported by both amdgpu and i915)
 - ycbcr420

In theory the auto option should choose the best available option for the
current setup, but because of bad internal conversion some monitors look
better with rgb and some with ycbcr444.

I looked at the patch and I couldn't find what is supposed to happen if
you set it to something else than auto, and the driver can't match that.
Are we supposed to fallback to the "auto" behaviour, or are we suppose
to reject the mode entirely?

The combination with the active output format property suggests the
former, but we should document it explicitly.

It is also my understanding that it should fall back to the "auto"
behaviour. I will add this to the documentation.


Yes, that was the intention, and then userspace can check, but it wasn't well 
received: https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_964530


Actually a lot of the thoughts that went into the original patch set can be 
found in that topic.


There was another iteration of the patch set that I never finished and sent to 
the LKML because I got discouraged by this: 
https://lore.kernel.org/dri-devel/20210623102923.70877c1a@eldfell/


I can try to dig it up, but it is completely untested and I don't think I still 
have the respective TODO list anymore, so I don't know if it is a better or 
worst starting point than the last iteration I sent to the LKML.


Greetings

Werner



Thanks,
Andri

Re: [PATCH 3/7] drm/amd/display: Add handling for new "active color format" property

[Werner Sembach]

Hi,

Am 10.01.24 um 14:09 schrieb Daniel Vetter:

On Wed, 10 Jan 2024 at 13:53, Andri Yngvason  wrote:

mið., 10. jan. 2024 kl. 11:10 skrifaði Daniel Vetter :

On Tue, Jan 09, 2024 at 06:11:00PM +, Andri Yngvason wrote:

+ /* Extract information from crtc to communicate it to userspace as 
connector properties */
+ for_each_new_connector_in_state(state, connector, new_con_state, i) {
+ struct drm_crtc *crtc = new_con_state->crtc;
+ struct dc_stream_state *stream;
+
+ if (crtc) {
+ new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
+ dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+ stream = dm_new_crtc_state->stream;
+
+ if (stream) {
+ 
drm_connector_set_active_color_format_property(connector,
+ 
convert_dc_pixel_encoding_into_drm_color_format(
+ 
dm_new_crtc_state->stream->timing.pixel_encoding));
+ }
+ } else {
+ drm_connector_set_active_color_format_property(connector, 
0);

Just realized an even bigger reason why your current design doesn't work:
You don't have locking here.

And you cannot grab the required lock, which is
drm_dev->mode_config.mutex, because that would result in deadlocks. So
this really needs to use the atomic state based design I've described.


Maybe we should just drop "actual color format" and instead fail the
modeset if the "preferred color format" property cannot be satisfied?
It seems like the simplest thing to do here, though it is perhaps less
convenient for userspace. In that case, the "preferred color format"
property should just be called "color format".

Yeah that's more in line with how other atomic properties work. This
way userspace can figure out what works with a TEST_ONLY commit too.
And for this to work you probably want to have an "automatic" setting
too.
-Sima


The problem with TEST_ONLY probing is that color format settings are 
interdependent: https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_966634


So changing any other setting may require every color format to be TEST_ONLY 
probed again.


Greetings

Werner

Re: [PATCH 3/7] drm/amd/display: Add handling for new "active color format" property

[Daniel Vetter]

On Wed, 10 Jan 2024 at 13:53, Andri Yngvason  wrote:
>
> mið., 10. jan. 2024 kl. 11:10 skrifaði Daniel Vetter :
> >
> > On Tue, Jan 09, 2024 at 06:11:00PM +, Andri Yngvason wrote:
> > > + /* Extract information from crtc to communicate it to userspace as 
> > > connector properties */
> > > + for_each_new_connector_in_state(state, connector, new_con_state, i) 
> > > {
> > > + struct drm_crtc *crtc = new_con_state->crtc;
> > > + struct dc_stream_state *stream;
> > > +
> > > + if (crtc) {
> > > + new_crtc_state = 
> > > drm_atomic_get_new_crtc_state(state, crtc);
> > > + dm_new_crtc_state = 
> > > to_dm_crtc_state(new_crtc_state);
> > > + stream = dm_new_crtc_state->stream;
> > > +
> > > + if (stream) {
> > > + 
> > > drm_connector_set_active_color_format_property(connector,
> > > + 
> > > convert_dc_pixel_encoding_into_drm_color_format(
> > > + 
> > > dm_new_crtc_state->stream->timing.pixel_encoding));
> > > + }
> > > + } else {
> > > + 
> > > drm_connector_set_active_color_format_property(connector, 0);
> >
> > Just realized an even bigger reason why your current design doesn't work:
> > You don't have locking here.
> >
> > And you cannot grab the required lock, which is
> > drm_dev->mode_config.mutex, because that would result in deadlocks. So
> > this really needs to use the atomic state based design I've described.
> >
>
> Maybe we should just drop "actual color format" and instead fail the
> modeset if the "preferred color format" property cannot be satisfied?
> It seems like the simplest thing to do here, though it is perhaps less
> convenient for userspace. In that case, the "preferred color format"
> property should just be called "color format".

Yeah that's more in line with how other atomic properties work. This
way userspace can figure out what works with a TEST_ONLY commit too.
And for this to work you probably want to have an "automatic" setting
too.
-Sima
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/1] drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl

[Zhang, Julia]

On 2024/1/10 18:47, Daniel Vetter wrote:
> On Thu, Dec 21, 2023 at 06:00:16PM +0800, Julia Zhang wrote:
>> From: Daniel Stone 
>>
>> Add a new ioctl to allow the guest VM to discover how the guest
>> actually allocated the underlying buffer, which allows buffers to
>> be used for GL<->Vulkan interop and through standard window systems.
>> It's also a step towards properly supporting modifiers in the guest.
>>
>> Signed-off-by: Daniel Stone 
>> Co-developed-by: Julia Zhang  # support query
>> stride before it's created
>> Signed-off-by: Julia Zhang 
>> ---
>>  drivers/gpu/drm/virtio/virtgpu_drv.c   |  1 +
>>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 22 -
>>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 66 ++
>>  drivers/gpu/drm/virtio/virtgpu_kms.c   |  8 +++-
>>  drivers/gpu/drm/virtio/virtgpu_vq.c| 63 
>>  include/uapi/drm/virtgpu_drm.h | 21 
>>  include/uapi/linux/virtio_gpu.h| 30 
>>  7 files changed, 208 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c 
>> b/drivers/gpu/drm/virtio/virtgpu_drv.c
>> index 4334c7608408..98061b714b98 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
>> @@ -148,6 +148,7 @@ static unsigned int features[] = {
>>  VIRTIO_GPU_F_RESOURCE_UUID,
>>  VIRTIO_GPU_F_RESOURCE_BLOB,
>>  VIRTIO_GPU_F_CONTEXT_INIT,
>> +VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT,
>>  };
>>  static struct virtio_driver virtio_gpu_driver = {
>>  .feature_table = features,
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
>> b/drivers/gpu/drm/virtio/virtgpu_drv.h
>> index 96365a772f77..bb5edcfeda54 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
>> @@ -214,6 +214,16 @@ struct virtio_gpu_drv_cap_cache {
>>  atomic_t is_valid;
>>  };
>>  
>> +struct virtio_gpu_query_info {
>> +uint32_t num_planes;
>> +uint64_t modifier;
>> +struct {
>> +uint64_t offset;
>> +uint32_t stride;
>> +} planes[VIRTIO_GPU_MAX_RESOURCE_PLANES];
>> +atomic_t is_valid;
>> +};
>> +
>>  struct virtio_gpu_device {
>>  struct drm_device *ddev;
>>  
>> @@ -246,6 +256,7 @@ struct virtio_gpu_device {
>>  bool has_resource_blob;
>>  bool has_host_visible;
>>  bool has_context_init;
>> +bool has_resource_query_layout;
>>  struct virtio_shm_region host_visible_region;
>>  struct drm_mm host_visible_mm;
>>  
>> @@ -277,7 +288,7 @@ struct virtio_gpu_fpriv {
>>  };
>>  
>>  /* virtgpu_ioctl.c */
>> -#define DRM_VIRTIO_NUM_IOCTLS 12
>> +#define DRM_VIRTIO_NUM_IOCTLS 13
>>  extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
>>  void virtio_gpu_create_context(struct drm_device *dev, struct drm_file 
>> *file);
>>  
>> @@ -420,6 +431,15 @@ virtio_gpu_cmd_set_scanout_blob(struct 
>> virtio_gpu_device *vgdev,
>>  uint32_t width, uint32_t height,
>>  uint32_t x, uint32_t y);
>>  
>> +int
>> +virtio_gpu_cmd_get_resource_layout(struct virtio_gpu_device *vgdev,
>> +   struct virtio_gpu_query_info *bo_info,
>> +   uint32_t width,
>> +   uint32_t height,
>> +   uint32_t format,
>> +   uint32_t bind,
>> +   uint32_t hw_res_handle);
>> +
>>  /* virtgpu_display.c */
>>  int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev);
>>  void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev);
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c 
>> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>> index b24b11f25197..216c04314177 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>> @@ -107,6 +107,9 @@ static int virtio_gpu_getparam_ioctl(struct drm_device 
>> *dev, void *data,
>>  case VIRTGPU_PARAM_SUPPORTED_CAPSET_IDs:
>>  value = vgdev->capset_id_mask;
>>  break;
>> +case VIRTGPU_PARAM_RESOURCE_QUERY_LAYOUT:
>> +value = vgdev->has_resource_query_layout ? 1 : 0;
>> +break;
>>  default:
>>  return -EINVAL;
>>  }
>> @@ -668,6 +671,65 @@ static int virtio_gpu_context_init_ioctl(struct 
>> drm_device *dev,
>>  return ret;
>>  }
>>  
>> +static int virtio_gpu_resource_query_layout_ioctl(struct drm_device *dev,
>> +  void *data,
>> +  struct drm_file *file)
>> +{
>> +struct drm_virtgpu_resource_query_layout *args = data;
>> +struct virtio_gpu_device *vgdev = dev->dev_private;
>> +struct drm_gem_object *obj = NULL;
>> +struct virtio_gpu_object *bo = NULL;
>> +struct virtio_gpu_query_info bo_info = {0};
>> +int ret = 0;
>> +int i;
>> +
>> +if 

Re: [PATCH 3/7] drm/amd/display: Add handling for new "active color format" property

[Daniel Vetter]

On Tue, Jan 09, 2024 at 06:11:00PM +, Andri Yngvason wrote:
> From: Werner Sembach 
> 
> This commit implements the "active color format" drm property for the AMD
> GPU driver.
> 
> Signed-off-by: Werner Sembach 
> Signed-off-by: Andri Yngvason 
> Tested-by: Andri Yngvason 
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++-
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 ++
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> 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 10e041a3b2545..b44d06c3b1706 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6882,6 +6882,24 @@ int convert_dc_color_depth_into_bpc(enum 
> dc_color_depth display_color_depth)
>   return 0;
>  }
>  
> +static int convert_dc_pixel_encoding_into_drm_color_format(
> + enum dc_pixel_encoding display_pixel_encoding)
> +{
> + switch (display_pixel_encoding) {
> + case PIXEL_ENCODING_RGB:
> + return DRM_COLOR_FORMAT_RGB444;
> + case PIXEL_ENCODING_YCBCR422:
> + return DRM_COLOR_FORMAT_YCBCR422;
> + case PIXEL_ENCODING_YCBCR444:
> + return DRM_COLOR_FORMAT_YCBCR444;
> + case PIXEL_ENCODING_YCBCR420:
> + return DRM_COLOR_FORMAT_YCBCR420;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
>  static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
> struct drm_crtc_state *crtc_state,
> struct drm_connector_state 
> *conn_state)
> @@ -7436,8 +7454,10 @@ void amdgpu_dm_connector_init_helper(struct 
> amdgpu_display_manager *dm,
>   adev->mode_info.underscan_vborder_property,
>   0);
>  
> - if (!aconnector->mst_root)
> + if (!aconnector->mst_root) {
>   drm_connector_attach_max_bpc_property(>base, 8, 16);
> + 
> drm_connector_attach_active_color_format_property(>base);
> + }
>  
>   aconnector->base.state->max_bpc = 16;
>   aconnector->base.state->max_requested_bpc = 
> aconnector->base.state->max_bpc;
> @@ -8969,6 +8989,26 @@ static void amdgpu_dm_atomic_commit_tail(struct 
> drm_atomic_state *state)
>   kfree(dummy_updates);
>   }
>  
> + /* Extract information from crtc to communicate it to userspace as 
> connector properties */
> + for_each_new_connector_in_state(state, connector, new_con_state, i) {
> + struct drm_crtc *crtc = new_con_state->crtc;
> + struct dc_stream_state *stream;
> +
> + if (crtc) {
> + new_crtc_state = drm_atomic_get_new_crtc_state(state, 
> crtc);
> + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> + stream = dm_new_crtc_state->stream;
> +
> + if (stream) {
> + 
> drm_connector_set_active_color_format_property(connector,
> + 
> convert_dc_pixel_encoding_into_drm_color_format(
> + 
> dm_new_crtc_state->stream->timing.pixel_encoding));
> + }
> + } else {
> + 
> drm_connector_set_active_color_format_property(connector, 0);

Just realized an even bigger reason why your current design doesn't work:
You don't have locking here.

And you cannot grab the required lock, which is
drm_dev->mode_config.mutex, because that would result in deadlocks. So
this really needs to use the atomic state based design I've described.

A bit a tanget, but it would be really good to add a lockdep assert into
drm_object_property_set_value, that at least for atomic drivers and
connectors the above lock must be held for changing property values. But
it will be quite a bit of audit to make sure all current users obey that
rule.

Cheers, Sima
> + }
> + }
> +
>   /**
>* Enable interrupts for CRTCs that are newly enabled or went through
>* a modeset. It was intentionally deferred until after the front end
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 11da0eebee6c4..a4d1b3ea8f81c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -600,6 +600,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
> *mgr,
>   if (connector->max_bpc_property)
>   drm_connector_attach_max_bpc_property(connector, 8, 16);
>  
> + connector->active_color_format_property = 
> master->base.active_color_format_property;
> + if (connector->active_color_format_property)
> + 
> 

RE: [PATCH 2/2] drm/amdgpu: Do bad page retirement for deferred errors

[Zhang, Hawking]

[AMD Official Use Only - General]

Let's drop the following message.

+   dev_info(adev->dev, "%ld uncorrectable hardware errors and "
+   "%ld deferred hardware errors detected in UMC 
block\n",
+   err_data->ue_count, err_data->de_count);

With that fixed, the series is

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: amd-gfx  On Behalf Of Candice Li
Sent: Wednesday, January 10, 2024 16:39
To: amd-gfx@lists.freedesktop.org
Cc: Li, Candice 
Subject: [PATCH 2/2] drm/amdgpu: Do bad page retirement for deferred errors

Needs to do bad page retirement for deferred errors.

Signed-off-by: Candice Li 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
index 848df7acdd3210..df61df7e9b155f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
@@ -93,6 +93,7 @@ static int amdgpu_umc_do_page_retirement(struct amdgpu_device 
*adev,
struct ras_err_data *err_data = (struct ras_err_data *)ras_error_status;
struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
int ret = 0;
+   unsigned long err_count;

kgd2kfd_set_sram_ecc_flag(adev->kfd.dev);
ret = amdgpu_dpm_get_ecc_info(adev, (void *)&(con->umc_ecc)); @@ 
-147,16 +148,17 @@ static int amdgpu_umc_do_page_retirement(struct 
amdgpu_device *adev,
}

/* only uncorrectable error needs gpu reset */
-   if (err_data->ue_count) {
-   dev_info(adev->dev, "%ld uncorrectable hardware errors "
-   "detected in UMC block\n",
-   err_data->ue_count);
+   if (err_data->ue_count || err_data->de_count) {
+   dev_info(adev->dev, "%ld uncorrectable hardware errors and "
+   "%ld deferred hardware errors detected in UMC 
block\n",
+   err_data->ue_count, err_data->de_count);

+   err_count = err_data->ue_count + err_data->de_count;
if ((amdgpu_bad_page_threshold != 0) &&
err_data->err_addr_cnt) {
amdgpu_ras_add_bad_pages(adev, err_data->err_addr,
err_data->err_addr_cnt);
-   amdgpu_ras_save_bad_pages(adev, &(err_data->ue_count));
+   amdgpu_ras_save_bad_pages(adev, _count);

amdgpu_dpm_send_hbm_bad_pages_num(adev, 
con->eeprom_control.ras_num_recs);

--
2.25.1

Re: [PATCH 1/1] drm/virtio: Implement device_attach

[Daniel Vetter]

On Wed, Jan 10, 2024 at 11:46:35AM +0100, Christian König wrote:
> Am 10.01.24 um 11:22 schrieb Daniel Vetter:
> > On Wed, Jan 10, 2024 at 11:19:37AM +0100, Christian König wrote:
> > > Am 10.01.24 um 10:56 schrieb Julia Zhang:
> > > > drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be
> > > > implemented, or else return ENOSYS. Virtio has no get_sg_table
> > > > implemented for vram object. To fix this, add a new device_attach to
> > > > call drm_gem_map_attach() for shmem object and return 0 for vram object
> > > > instead of calling drm_gem_map_attach for both of these two kinds of
> > > > object.
> > > Well as far as I can see this is nonsense from the DMA-buf side of things.
> > > 
> > > SG tables are always needed as long as you don't re-import the same object
> > > into your driver and then you shouldn't end up in this function in the 
> > > first
> > > place.
> > > 
> > > So that drm_gem_map_attach() requires get_sg_table to be implemented is
> > > intentional and should never be overridden like this.
> > See my reply, tldr; you're allowed to reject ->attach with -EBUSY to
> > handle exactly this case of non-shareable buffer types. But definitely
> > don't silently fail, that's a "we'll oops on map_attachment" kind of bug
> > :-)
> 
> Ah, yes that makes much more sense!
> 
> So basically just the "return 0;" needs to be "return -EBUSY;".

Well plus 2nd patch to polish the virtio_dma_buf docs a bit, that would be
nice :-D
-Sima

> 
> Regards,
> Christian.
> 
> > -Sima
> > 
> > > Regards,
> > > Christian.
> > > 
> > > > Signed-off-by: Julia Zhang 
> > > > ---
> > > >drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +-
> > > >1 file changed, 13 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c 
> > > > b/drivers/gpu/drm/virtio/virtgpu_prime.c
> > > > index 44425f20d91a..f0b0ff6f3813 100644
> > > > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> > > > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> > > > @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct 
> > > > dma_buf_attachment *attach,
> > > > drm_gem_unmap_dma_buf(attach, sgt, dir);
> > > >}
> > > > +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf,
> > > > +struct dma_buf_attachment *attach)
> > > > +{
> > > > +   struct drm_gem_object *obj = attach->dmabuf->priv;
> > > > +   struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
> > > > +
> > > > +   if (virtio_gpu_is_vram(bo))
> > > > +   return 0;
> > > > +
> > > > +   return drm_gem_map_attach(dma_buf, attach);
> > > > +}
> > > > +
> > > >static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
> > > > .ops = {
> > > > .cache_sgt_mapping = true,
> > > > @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops 
> > > > virtgpu_dmabuf_ops =  {
> > > > .vmap = drm_gem_dmabuf_vmap,
> > > > .vunmap = drm_gem_dmabuf_vunmap,
> > > > },
> > > > -   .device_attach = drm_gem_map_attach,
> > > > +   .device_attach = virtgpu_gem_device_attach,
> > > > .get_uuid = virtgpu_virtio_get_uuid,
> > > >};
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/1] drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl

[Daniel Vetter]

On Thu, Dec 21, 2023 at 06:00:16PM +0800, Julia Zhang wrote:
> From: Daniel Stone 
> 
> Add a new ioctl to allow the guest VM to discover how the guest
> actually allocated the underlying buffer, which allows buffers to
> be used for GL<->Vulkan interop and through standard window systems.
> It's also a step towards properly supporting modifiers in the guest.
> 
> Signed-off-by: Daniel Stone 
> Co-developed-by: Julia Zhang  # support query
> stride before it's created
> Signed-off-by: Julia Zhang 
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.c   |  1 +
>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 22 -
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 66 ++
>  drivers/gpu/drm/virtio/virtgpu_kms.c   |  8 +++-
>  drivers/gpu/drm/virtio/virtgpu_vq.c| 63 
>  include/uapi/drm/virtgpu_drm.h | 21 
>  include/uapi/linux/virtio_gpu.h| 30 
>  7 files changed, 208 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c 
> b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index 4334c7608408..98061b714b98 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -148,6 +148,7 @@ static unsigned int features[] = {
>   VIRTIO_GPU_F_RESOURCE_UUID,
>   VIRTIO_GPU_F_RESOURCE_BLOB,
>   VIRTIO_GPU_F_CONTEXT_INIT,
> + VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT,
>  };
>  static struct virtio_driver virtio_gpu_driver = {
>   .feature_table = features,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 96365a772f77..bb5edcfeda54 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -214,6 +214,16 @@ struct virtio_gpu_drv_cap_cache {
>   atomic_t is_valid;
>  };
>  
> +struct virtio_gpu_query_info {
> + uint32_t num_planes;
> + uint64_t modifier;
> + struct {
> + uint64_t offset;
> + uint32_t stride;
> + } planes[VIRTIO_GPU_MAX_RESOURCE_PLANES];
> + atomic_t is_valid;
> +};
> +
>  struct virtio_gpu_device {
>   struct drm_device *ddev;
>  
> @@ -246,6 +256,7 @@ struct virtio_gpu_device {
>   bool has_resource_blob;
>   bool has_host_visible;
>   bool has_context_init;
> + bool has_resource_query_layout;
>   struct virtio_shm_region host_visible_region;
>   struct drm_mm host_visible_mm;
>  
> @@ -277,7 +288,7 @@ struct virtio_gpu_fpriv {
>  };
>  
>  /* virtgpu_ioctl.c */
> -#define DRM_VIRTIO_NUM_IOCTLS 12
> +#define DRM_VIRTIO_NUM_IOCTLS 13
>  extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
>  void virtio_gpu_create_context(struct drm_device *dev, struct drm_file 
> *file);
>  
> @@ -420,6 +431,15 @@ virtio_gpu_cmd_set_scanout_blob(struct virtio_gpu_device 
> *vgdev,
>   uint32_t width, uint32_t height,
>   uint32_t x, uint32_t y);
>  
> +int
> +virtio_gpu_cmd_get_resource_layout(struct virtio_gpu_device *vgdev,
> +struct virtio_gpu_query_info *bo_info,
> +uint32_t width,
> +uint32_t height,
> +uint32_t format,
> +uint32_t bind,
> +uint32_t hw_res_handle);
> +
>  /* virtgpu_display.c */
>  int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev);
>  void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c 
> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index b24b11f25197..216c04314177 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -107,6 +107,9 @@ static int virtio_gpu_getparam_ioctl(struct drm_device 
> *dev, void *data,
>   case VIRTGPU_PARAM_SUPPORTED_CAPSET_IDs:
>   value = vgdev->capset_id_mask;
>   break;
> + case VIRTGPU_PARAM_RESOURCE_QUERY_LAYOUT:
> + value = vgdev->has_resource_query_layout ? 1 : 0;
> + break;
>   default:
>   return -EINVAL;
>   }
> @@ -668,6 +671,65 @@ static int virtio_gpu_context_init_ioctl(struct 
> drm_device *dev,
>   return ret;
>  }
>  
> +static int virtio_gpu_resource_query_layout_ioctl(struct drm_device *dev,
> +   void *data,
> +   struct drm_file *file)
> +{
> + struct drm_virtgpu_resource_query_layout *args = data;
> + struct virtio_gpu_device *vgdev = dev->dev_private;
> + struct drm_gem_object *obj = NULL;
> + struct virtio_gpu_object *bo = NULL;
> + struct virtio_gpu_query_info bo_info = {0};
> + int ret = 0;
> + int i;
> +
> + if (!vgdev->has_resource_query_layout) {
> + DRM_ERROR("failing: no RQL on host\n");
> + return -EINVAL;
> + }
> +
> + 

Re: [PATCH 1/1] drm/virtio: Implement device_attach

[Christian König]

Am 10.01.24 um 11:22 schrieb Daniel Vetter:

On Wed, Jan 10, 2024 at 11:19:37AM +0100, Christian König wrote:

Am 10.01.24 um 10:56 schrieb Julia Zhang:

drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be
implemented, or else return ENOSYS. Virtio has no get_sg_table
implemented for vram object. To fix this, add a new device_attach to
call drm_gem_map_attach() for shmem object and return 0 for vram object
instead of calling drm_gem_map_attach for both of these two kinds of
object.

Well as far as I can see this is nonsense from the DMA-buf side of things.

SG tables are always needed as long as you don't re-import the same object
into your driver and then you shouldn't end up in this function in the first
place.

So that drm_gem_map_attach() requires get_sg_table to be implemented is
intentional and should never be overridden like this.

See my reply, tldr; you're allowed to reject ->attach with -EBUSY to
handle exactly this case of non-shareable buffer types. But definitely
don't silently fail, that's a "we'll oops on map_attachment" kind of bug
:-)


Ah, yes that makes much more sense!

So basically just the "return 0;" needs to be "return -EBUSY;".

Regards,
Christian.


-Sima


Regards,
Christian.


Signed-off-by: Julia Zhang 
---
   drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +-
   1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c 
b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 44425f20d91a..f0b0ff6f3813 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct 
dma_buf_attachment *attach,
drm_gem_unmap_dma_buf(attach, sgt, dir);
   }
+static int virtgpu_gem_device_attach(struct dma_buf *dma_buf,
+struct dma_buf_attachment *attach)
+{
+   struct drm_gem_object *obj = attach->dmabuf->priv;
+   struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
+
+   if (virtio_gpu_is_vram(bo))
+   return 0;
+
+   return drm_gem_map_attach(dma_buf, attach);
+}
+
   static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
.ops = {
.cache_sgt_mapping = true,
@@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
.vmap = drm_gem_dmabuf_vmap,
.vunmap = drm_gem_dmabuf_vunmap,
},
-   .device_attach = drm_gem_map_attach,
+   .device_attach = virtgpu_gem_device_attach,
.get_uuid = virtgpu_virtio_get_uuid,
   };

Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace

[Daniel Vetter]

On Tue, Jan 09, 2024 at 11:12:11PM +, Andri Yngvason wrote:
> Hi Daniel,
> 
> þri., 9. jan. 2024 kl. 22:32 skrifaði Daniel Stone :
> 
> > On Tue, 9 Jan 2024 at 18:12, Andri Yngvason  wrote:
> > > + * active color format:
> > > + * This read-only property tells userspace the color format
> > 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. Drivers shall use
> > > + * drm_connector_attach_active_color_format_property() to install
> > this
> > > + * property. Possible values are "not applicable", "rgb",
> > "ycbcr444",
> > > + * "ycbcr422", and "ycbcr420".
> >
> > How does userspace determine what's happened without polling? Will it
> > only change after an `ALLOW_MODESET` commit, and be guaranteed to be
> > updated after the commit has completed and the event being sent?
> > Should it send a HOTPLUG event? Other?
> >
> 
> Userspace does not determine what's happened without polling. The purpose
> of this property is not for programmatic verification that the preferred
> property was applied. It is my understanding that it's mostly intended for
> debugging purposes. It should only change as a consequence of modesetting,
> although I didn't actually look into what happens if you set the "preferred
> color format" outside of a modeset.

This feels a bit irky to me, since we don't have any synchronization and
it kinda breaks how userspace gets to know about stuff.

For context the current immutable properties are all stuff that's derived
from the sink (like edid, or things like that). Userspace is guaranteed to
get a hotplug event (minus driver bugs as usual) if any of these change,
and we've added infrastructure so that the hotplug event even contains the
specific property so that userspace can avoid re-read (which can cause
some costly re-probing) them all.

As an example you can look at drm_connector_set_link_status_property,
which drivers follow by a call to drm_kms_helper_connector_hotplug_event
to make sure userspace knows about what's up. Could be optimized I think.

This thing here works entirely differently, and I think we need somewhat
new semantics for this:

- I agree it should be read-only for userspace, so immutable sounds right.

- But I also agree with Daniel Stone that this should be tied more
  directly to the modeset state.

So I think the better approach would be to put the output type into
drm_connector_state, require that drivers compute it in their
->atomic_check code (which in the future would allow us to report it out
for TEST_ONLY commits too), and so guarantee that the value is updated
right after the kms ioctl returns (and not somewhen later for non-blocking
commits).

You probably need a bit of work to be able to handle immutable properties
with the atomic state infrastructure, but I think otherwise this should
fit all rather neatly.

Cheers, Sima
> 
> The way I've implemented things in sway, calling the
> "preferred_signal_format" command triggers a modeset with the "preferred
> color format" set and calling "get_outputs", immediately queries the
> "actual color format" and displays it.
> 
> Regards,
> Andri

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/1] drm/virtio: Implement device_attach

[Daniel Vetter]

On Wed, Jan 10, 2024 at 11:19:37AM +0100, Christian König wrote:
> Am 10.01.24 um 10:56 schrieb Julia Zhang:
> > drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be
> > implemented, or else return ENOSYS. Virtio has no get_sg_table
> > implemented for vram object. To fix this, add a new device_attach to
> > call drm_gem_map_attach() for shmem object and return 0 for vram object
> > instead of calling drm_gem_map_attach for both of these two kinds of
> > object.
> 
> Well as far as I can see this is nonsense from the DMA-buf side of things.
> 
> SG tables are always needed as long as you don't re-import the same object
> into your driver and then you shouldn't end up in this function in the first
> place.
> 
> So that drm_gem_map_attach() requires get_sg_table to be implemented is
> intentional and should never be overridden like this.

See my reply, tldr; you're allowed to reject ->attach with -EBUSY to
handle exactly this case of non-shareable buffer types. But definitely
don't silently fail, that's a "we'll oops on map_attachment" kind of bug
:-)
-Sima

> 
> Regards,
> Christian.
> 
> > 
> > Signed-off-by: Julia Zhang 
> > ---
> >   drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +-
> >   1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c 
> > b/drivers/gpu/drm/virtio/virtgpu_prime.c
> > index 44425f20d91a..f0b0ff6f3813 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> > @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct 
> > dma_buf_attachment *attach,
> > drm_gem_unmap_dma_buf(attach, sgt, dir);
> >   }
> > +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf,
> > +struct dma_buf_attachment *attach)
> > +{
> > +   struct drm_gem_object *obj = attach->dmabuf->priv;
> > +   struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
> > +
> > +   if (virtio_gpu_is_vram(bo))
> > +   return 0;
> > +
> > +   return drm_gem_map_attach(dma_buf, attach);
> > +}
> > +
> >   static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
> > .ops = {
> > .cache_sgt_mapping = true,
> > @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops 
> > =  {
> > .vmap = drm_gem_dmabuf_vmap,
> > .vunmap = drm_gem_dmabuf_vunmap,
> > },
> > -   .device_attach = drm_gem_map_attach,
> > +   .device_attach = virtgpu_gem_device_attach,
> > .get_uuid = virtgpu_virtio_get_uuid,
> >   };
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/1] drm/virtio: Implement device_attach

[Daniel Vetter]

On Wed, Jan 10, 2024 at 05:56:28PM +0800, Julia Zhang wrote:
> drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be
> implemented, or else return ENOSYS. Virtio has no get_sg_table
> implemented for vram object. To fix this, add a new device_attach to
> call drm_gem_map_attach() for shmem object and return 0 for vram object
> instead of calling drm_gem_map_attach for both of these two kinds of
> object.
> 
> Signed-off-by: Julia Zhang 
> ---
>  drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c 
> b/drivers/gpu/drm/virtio/virtgpu_prime.c
> index 44425f20d91a..f0b0ff6f3813 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct 
> dma_buf_attachment *attach,
>   drm_gem_unmap_dma_buf(attach, sgt, dir);
>  }
>  
> +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf,
> +  struct dma_buf_attachment *attach)
> +{
> + struct drm_gem_object *obj = attach->dmabuf->priv;
> + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
> +
> + if (virtio_gpu_is_vram(bo))
> + return 0;

You need to reject attach here because these vram buffer objects cannot be
used by any other driver. In that case dma_buf_attach _must_ fail, not
silently succeed.

Because if it silently succeeds then the subsequent dma_buf_map_attachment
will blow up because you don't have the ->get_sg_table hook implemented.

Per the documentation the error code for this case must be -EBUSY, see the
section for the attach hook here:

https://dri.freedesktop.org/docs/drm/driver-api/dma-buf.html#c.dma_buf_ops

Since you're looking into this area, please make sure there's not other
similar mistake in virtio code.

Also can you please make a kerneldoc patch for struct virtio_dma_buf_ops
to improve the documentation there? I think it would be good to move those
to the inline style and then at least put a kernel-doc hyperlink to struct
dma_buf_ops.attach and mention that attach must fail for non-shareable
buffers.

In general the virtio_dma_buf kerneldoc seems to be on the "too minimal,
explains nothing" side of things :-/

Cheers, Sima

> +
> + return drm_gem_map_attach(dma_buf, attach);
> +}
> +
>  static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
>   .ops = {
>   .cache_sgt_mapping = true,
> @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = 
>  {
>   .vmap = drm_gem_dmabuf_vmap,
>   .vunmap = drm_gem_dmabuf_vunmap,
>   },
> - .device_attach = drm_gem_map_attach,
> + .device_attach = virtgpu_gem_device_attach,
>   .get_uuid = virtgpu_virtio_get_uuid,
>  };
>  
> -- 
> 2.34.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v3] drm/amdgpu: Check extended configuration space register when system uses large bar

[Christian König]

Am 10.01.24 um 11:13 schrieb Ma Jun:

Some customer platforms do not enable mmconfig for various reasons,
such as bios bug, and therefore cannot access the GPU extend configuration
space through mmio.

When the system enters the d3cold state and resumes, the amdgpu driver
fails to resume because the extend configuration space registers of
GPU can't be restored. At this point, Usually we only see some failure
dmesg log printed by amdgpu driver, it is difficult to find the root
cause.

Therefor print a warnning message if the system can't access the
extended configuration space register when using large bar.

Signed-off-by: Ma Jun 


Reviewed-by: Christian König 


---
v2:
- Check the register at 0x100 but not resize bar register
- Modify the commit message
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 4b694696930e..629de7f2908c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1442,6 +1442,10 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device 
*adev)
if (amdgpu_sriov_vf(adev))
return 0;
  
+	/* PCI_EXT_CAP_ID_VNDR extended capability is located at 0x100 */

+   if (!pci_find_ext_capability(adev->pdev, PCI_EXT_CAP_ID_VNDR))
+   DRM_WARN("System can't access extended configuration space,please 
check!!\n");
+
/* skip if the bios has already enabled large BAR */
if (adev->gmc.real_vram_size &&
(pci_resource_len(adev->pdev, 0) >= adev->gmc.real_vram_size))

Re: [PATCH 1/1] drm/virtio: Implement device_attach

[Christian König]

Am 10.01.24 um 10:56 schrieb Julia Zhang:

drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be
implemented, or else return ENOSYS. Virtio has no get_sg_table
implemented for vram object. To fix this, add a new device_attach to
call drm_gem_map_attach() for shmem object and return 0 for vram object
instead of calling drm_gem_map_attach for both of these two kinds of
object.


Well as far as I can see this is nonsense from the DMA-buf side of things.

SG tables are always needed as long as you don't re-import the same 
object into your driver and then you shouldn't end up in this function 
in the first place.


So that drm_gem_map_attach() requires get_sg_table to be implemented is 
intentional and should never be overridden like this.


Regards,
Christian.



Signed-off-by: Julia Zhang 
---
  drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +-
  1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c 
b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 44425f20d91a..f0b0ff6f3813 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct 
dma_buf_attachment *attach,
drm_gem_unmap_dma_buf(attach, sgt, dir);
  }
  
+static int virtgpu_gem_device_attach(struct dma_buf *dma_buf,

+struct dma_buf_attachment *attach)
+{
+   struct drm_gem_object *obj = attach->dmabuf->priv;
+   struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
+
+   if (virtio_gpu_is_vram(bo))
+   return 0;
+
+   return drm_gem_map_attach(dma_buf, attach);
+}
+
  static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
.ops = {
.cache_sgt_mapping = true,
@@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
.vmap = drm_gem_dmabuf_vmap,
.vunmap = drm_gem_dmabuf_vunmap,
},
-   .device_attach = drm_gem_map_attach,
+   .device_attach = virtgpu_gem_device_attach,
.get_uuid = virtgpu_virtio_get_uuid,
  };
  

[PATCH v3] drm/amdgpu: Check extended configuration space register when system uses large bar

[Ma Jun]

Some customer platforms do not enable mmconfig for various reasons,
such as bios bug, and therefore cannot access the GPU extend configuration
space through mmio.

When the system enters the d3cold state and resumes, the amdgpu driver
fails to resume because the extend configuration space registers of
GPU can't be restored. At this point, Usually we only see some failure
dmesg log printed by amdgpu driver, it is difficult to find the root
cause.

Therefor print a warnning message if the system can't access the
extended configuration space register when using large bar.

Signed-off-by: Ma Jun 
---
v2:
- Check the register at 0x100 but not resize bar register
- Modify the commit message
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 4b694696930e..629de7f2908c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1442,6 +1442,10 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device 
*adev)
if (amdgpu_sriov_vf(adev))
return 0;
 
+   /* PCI_EXT_CAP_ID_VNDR extended capability is located at 0x100 */
+   if (!pci_find_ext_capability(adev->pdev, PCI_EXT_CAP_ID_VNDR))
+   DRM_WARN("System can't access extended configuration 
space,please check!!\n");
+
/* skip if the bios has already enabled large BAR */
if (adev->gmc.real_vram_size &&
(pci_resource_len(adev->pdev, 0) >= adev->gmc.real_vram_size))
-- 
2.34.1

[PATCH 1/1] drm/virtio: Implement device_attach

[Julia Zhang]

drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be
implemented, or else return ENOSYS. Virtio has no get_sg_table
implemented for vram object. To fix this, add a new device_attach to
call drm_gem_map_attach() for shmem object and return 0 for vram object
instead of calling drm_gem_map_attach for both of these two kinds of
object.

Signed-off-by: Julia Zhang 
---
 drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c 
b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 44425f20d91a..f0b0ff6f3813 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct 
dma_buf_attachment *attach,
drm_gem_unmap_dma_buf(attach, sgt, dir);
 }
 
+static int virtgpu_gem_device_attach(struct dma_buf *dma_buf,
+struct dma_buf_attachment *attach)
+{
+   struct drm_gem_object *obj = attach->dmabuf->priv;
+   struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
+
+   if (virtio_gpu_is_vram(bo))
+   return 0;
+
+   return drm_gem_map_attach(dma_buf, attach);
+}
+
 static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
.ops = {
.cache_sgt_mapping = true,
@@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops =  {
.vmap = drm_gem_dmabuf_vmap,
.vunmap = drm_gem_dmabuf_vunmap,
},
-   .device_attach = drm_gem_map_attach,
+   .device_attach = virtgpu_gem_device_attach,
.get_uuid = virtgpu_virtio_get_uuid,
 };
 
-- 
2.34.1

[PATCH 0/1] Implement device_attach for virtio gpu

[Julia Zhang]

To realize dGPU prime feature for virtio gpu, we are trying let dGPU import
vram object of virtio gpu. But this feature would finally call function
virtio_dma_buf_ops.device_attach(), which was set as drm_gem_map_attach().
drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be
implemented, or else return ENOSYS. But virtio gpu driver has not
implemented it for vram object and actually vram object does not require
it. So this add a new implementation of device_attach() to call
drm_gem_map_attach() for shmem object and return 0 for vram object as it
actually did before the requirement was added.

Julia Zhang (1):
  drm/virtio: Implement device_attach

 drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

-- 
2.34.1

Re: [PATCH v2 0/1] Implementation of resource_query_layout

[Zhang, Julia]

I see, I will implement this. Thank you very much.

On 2023/12/27 09:29, Gurchetan Singh wrote:
> 
> 
> On Thu, Dec 21, 2023 at 2:01 AM Julia Zhang  > wrote:
> 
> Hi all,
> 
> Sorry to late reply. This is v2 of the implementation of
> resource_query_layout. This adds a new ioctl to let guest query 
> information
> of host resource, which is originally from Daniel Stone. We add some
> changes to support query the correct stride of host resource before it's
> created, which is to support to blit data from dGPU to virtio iGPU for 
> dGPU
> prime feature.
> 
> Changes from v1 to v2:
> -Squash two patches to a single patch.
> -A small modification of VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT
> 
> 
> Below is description of v1:
> This add implementation of resource_query_layout to get the information of
> how the host has actually allocated the buffer. This function is now used
> to query the stride for guest linear resource for dGPU prime on guest VMs.
> 
> 
> You can use a context specific protocol or even the virgl capabilities [for a 
> linear strided resource].  For example, Sommelier does the following:
> 
> https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/vm_tools/sommelier/virtualization/virtgpu_channel.cc#549
>  
> 
> 
> i.e, you should be able to avoid extra ioctl + hypercall.
>  
> 
> 
> v1 of kernel side:
>  https:
> 
> //lore.kernel.org/xen-devel/20231110074027.24862-1-julia.zh...@amd.com/T/#t 
> 
> 
> v1 of qemu side:
> https:
> 
> //lore.kernel.org/qemu-devel/20231110074027.24862-1-julia.zh...@amd.com/T/#t 
> 
> 
> Daniel Stone (1):
>   drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl
> 
>  drivers/gpu/drm/virtio/virtgpu_drv.c   |  1 +
>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 22 -
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 66 ++
>  drivers/gpu/drm/virtio/virtgpu_kms.c   |  8 +++-
>  drivers/gpu/drm/virtio/virtgpu_vq.c    | 63 
>  include/uapi/drm/virtgpu_drm.h         | 21 
>  include/uapi/linux/virtio_gpu.h        | 30 
>  7 files changed, 208 insertions(+), 3 deletions(-)
> 
> -- 
> 2.34.1
> 

[PATCH] drm/amdkfd: Fix the shift-out-of-bounds warning

[Ma Jun]

There is following shift-out-of-bounds warning if ecode=0.
"shift exponent 4294967295 is too large for 64-bit type 'long long unsigned 
int'"

Signed-off-by: Ma Jun 
---
 include/uapi/linux/kfd_ioctl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index 2aa88afe305b..129325b02a91 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -1004,7 +1004,7 @@ enum kfd_dbg_trap_exception_code {
 };
 
 /* Mask generated by ecode in kfd_dbg_trap_exception_code */
-#define KFD_EC_MASK(ecode) (1ULL << (ecode - 1))
+#define KFD_EC_MASK(ecode) (BIT(ecode) - 1)
 
 /* Masks for exception code type checks below */
 #define KFD_EC_MASK_QUEUE  (KFD_EC_MASK(EC_QUEUE_WAVE_ABORT) | \
-- 
2.34.1

Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace

[Maxime Ripard]

Hi,

On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote:
> From: Werner Sembach 
> 
> Add a new general drm property "preferred color format" which can be used
> by userspace to tell the graphic drivers to which color format to use.
> 
> Possible options are:
> - auto (default/current behaviour)
> - rgb
> - ycbcr444
> - ycbcr422 (not supported by both amdgpu and i915)
> - ycbcr420
> 
> In theory the auto option should choose the best available option for the
> current setup, but because of bad internal conversion some monitors look
> better with rgb and some with ycbcr444.

I looked at the patch and I couldn't find what is supposed to happen if
you set it to something else than auto, and the driver can't match that.
Are we supposed to fallback to the "auto" behaviour, or are we suppose
to reject the mode entirely?

The combination with the active output format property suggests the
former, but we should document it explicitly.

> Also, because of bad shielded connectors and/or cables, it might be
> preferable to use the less bandwidth heavy ycbcr422 and ycbcr420 formats
> for a signal that is less deceptible to interference.
> 
> In the future, automatic color calibration for screens might also depend on
> this option being available.
> 
> Signed-off-by: Werner Sembach 
> Reported-by: Dan Carpenter 
> Signed-off-by: Andri Yngvason 
> Tested-by: Andri Yngvason 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c |  4 +++
>  drivers/gpu/drm/drm_atomic_uapi.c   |  4 +++
>  drivers/gpu/drm/drm_connector.c | 50 -
>  include/drm/drm_connector.h | 17 ++
>  4 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 68ffcc0b00dca..745a43d9c5da3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -707,6 +707,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>   if (old_connector_state->max_requested_bpc !=
>   new_connector_state->max_requested_bpc)
>   new_crtc_state->connectors_changed = true;
> +
> + if (old_connector_state->preferred_color_format !=
> + new_connector_state->preferred_color_format)
> + new_crtc_state->connectors_changed = true;
>   }
>  
>   if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 98d3b10c08ae1..eba5dea1249e5 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -798,6 +798,8 @@ static int drm_atomic_connector_set_property(struct 
> drm_connector *connector,
>   state->max_requested_bpc = val;
>   } else if (property == connector->privacy_screen_sw_state_property) {
>   state->privacy_screen_sw_state = val;
> + } else if (property == connector->preferred_color_format_property) {
> + state->preferred_color_format = val;
>   } else if (connector->funcs->atomic_set_property) {
>   return connector->funcs->atomic_set_property(connector,
>   state, property, val);
> @@ -881,6 +883,8 @@ drm_atomic_connector_get_property(struct drm_connector 
> *connector,
>   *val = state->max_requested_bpc;
>   } else if (property == connector->privacy_screen_sw_state_property) {
>   *val = state->privacy_screen_sw_state;
> + } else if (property == connector->preferred_color_format_property) {
> + *val = state->preferred_color_format;
>   } 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 30d62e505d188..4de48a38792cf 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1061,6 +1061,14 @@ static const struct drm_prop_enum_list 
> drm_dp_subconnector_enum_list[] = {
>   { DRM_MODE_SUBCONNECTOR_Native,  "Native"}, /* DP */
>  };
>  
> +static const struct drm_prop_enum_list 
> drm_preferred_color_format_enum_list[] = {
> + { 0, "auto" },
> + { DRM_COLOR_FORMAT_RGB444, "rgb" },
> + { DRM_COLOR_FORMAT_YCBCR444, "ycbcr444" },
> + { DRM_COLOR_FORMAT_YCBCR422, "ycbcr422" },
> + { DRM_COLOR_FORMAT_YCBCR420, "ycbcr420" },
> +};
> +
>  static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = 
> {
>   { 0, "not applicable" },
>   { DRM_COLOR_FORMAT_RGB444, "rgb" },
> @@ -1398,11 +1406,20 @@ static const u32 dp_colorspaces =
>   *   drm_connector_attach_max_bpc_property() to create and attach the
>   *   property to the connector during initialization.
>   *
> + * preferred 

Re: [PATCH 3/3] drm/amdgpu: move debug options init prior to amdgpu device init

[Christian König]

Am 09.01.24 um 11:13 schrieb Le Ma:

To bring debug options into effect in early initialization phase

Signed-off-by: Le Ma 


Reviewed-by: Christian König  for this one and 
#1 in the series.



---
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3e0e39a1b5ba..b67ffc3a9a3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2234,6 +2234,8 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
  
  	pci_set_drvdata(pdev, ddev);
  
+	amdgpu_init_debug_options(adev);

+
ret = amdgpu_driver_load_kms(adev, flags);
if (ret)
goto err_pci;
@@ -2314,8 +2316,6 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
amdgpu_get_secondary_funcs(adev);
}
  
-	amdgpu_init_debug_options(adev);

-
return 0;
  
  err_pci:

Re: [PATCH 2/3 v2] drm/amdgpu: add debug flag to place fw bo on vram for frontdoor loading

[Christian König]

Am 09.01.24 um 14:00 schrieb Le Ma:

Use debug_mask=0x8 param to help isolating data path issues
on new systems in early phase.

v2: rename the flag for explicitness (lijo)

Signed-off-by: Le Ma 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 6 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   | 2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 3 ++-
  4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 616b6c911767..3d8a48f46b01 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1144,6 +1144,7 @@ struct amdgpu_device {
booldebug_vm;
booldebug_largebar;
booldebug_disable_soft_recovery;
+   booldebug_use_vram_fw_buf;
  };
  
  static inline uint32_t amdgpu_ip_version(const struct amdgpu_device *adev,

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 880137774b4e..0776b0c5e4e4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -128,6 +128,7 @@ enum AMDGPU_DEBUG_MASK {
AMDGPU_DEBUG_VM = BIT(0),
AMDGPU_DEBUG_LARGEBAR = BIT(1),
AMDGPU_DEBUG_DISABLE_GPU_SOFT_RECOVERY = BIT(2),
+   AMDGPU_DEBUG_USE_VRAM_FW_BUF = BIT(3),
  };
  
  unsigned int amdgpu_vram_limit = UINT_MAX;

@@ -2117,6 +2118,11 @@ static void amdgpu_init_debug_options(struct 
amdgpu_device *adev)
pr_info("debug: soft reset for GPU recovery disabled\n");
adev->debug_disable_soft_recovery = true;
}
+
+   if (amdgpu_debug_mask & AMDGPU_DEBUG_USE_VRAM_FW_BUF) {
+   pr_info("debug: place fw in vram for frontdoor loading\n");
+   adev->debug_use_vram_fw_buf = true;
+   }
  }
  
  static unsigned long amdgpu_fix_asic_type(struct pci_dev *pdev, unsigned long flags)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51bfe3757c89..215994409ac1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -467,7 +467,7 @@ static int psp_sw_init(void *handle)
}
  
  	ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,

- amdgpu_sriov_vf(adev) ?
+ (amdgpu_sriov_vf(adev) || 
adev->debug_use_vram_fw_buf) ?
  AMDGPU_GEM_DOMAIN_VRAM : 
AMDGPU_GEM_DOMAIN_GTT,
  >fw_pri_bo,
  >fw_pri_mc_addr,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 0efb2568cb65..3e12763e477a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -1062,7 +1062,8 @@ int amdgpu_ucode_create_bo(struct amdgpu_device *adev)
  {
if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) {
amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
-   amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : 
AMDGPU_GEM_DOMAIN_GTT,
+   (amdgpu_sriov_vf(adev) || adev->debug_use_vram_fw_buf) ?
+   AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
>firmware.fw_buf,
>firmware.fw_buf_mc,
>firmware.fw_buf_ptr);

[PATCH 2/2] drm/amdgpu: Do bad page retirement for deferred errors

[Candice Li]

Needs to do bad page retirement for deferred errors.

Signed-off-by: Candice Li 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
index 848df7acdd3210..df61df7e9b155f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
@@ -93,6 +93,7 @@ static int amdgpu_umc_do_page_retirement(struct amdgpu_device 
*adev,
struct ras_err_data *err_data = (struct ras_err_data *)ras_error_status;
struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
int ret = 0;
+   unsigned long err_count;
 
kgd2kfd_set_sram_ecc_flag(adev->kfd.dev);
ret = amdgpu_dpm_get_ecc_info(adev, (void *)&(con->umc_ecc));
@@ -147,16 +148,17 @@ static int amdgpu_umc_do_page_retirement(struct 
amdgpu_device *adev,
}
 
/* only uncorrectable error needs gpu reset */
-   if (err_data->ue_count) {
-   dev_info(adev->dev, "%ld uncorrectable hardware errors "
-   "detected in UMC block\n",
-   err_data->ue_count);
+   if (err_data->ue_count || err_data->de_count) {
+   dev_info(adev->dev, "%ld uncorrectable hardware errors and "
+   "%ld deferred hardware errors detected in UMC 
block\n",
+   err_data->ue_count, err_data->de_count);
 
+   err_count = err_data->ue_count + err_data->de_count;
if ((amdgpu_bad_page_threshold != 0) &&
err_data->err_addr_cnt) {
amdgpu_ras_add_bad_pages(adev, err_data->err_addr,
err_data->err_addr_cnt);
-   amdgpu_ras_save_bad_pages(adev, &(err_data->ue_count));
+   amdgpu_ras_save_bad_pages(adev, _count);
 
amdgpu_dpm_send_hbm_bad_pages_num(adev, 
con->eeprom_control.ras_num_recs);
 
-- 
2.25.1

[PATCH 1/2] drm/amdgpu: Log deferred error separately

[Candice Li]

Separate deferred error from UE and CE and log it
individually.

Signed-off-by: Candice Li 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c   |  11 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c   | 116 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h   |   6 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c   |   1 +
 drivers/gpu/drm/amd/amdgpu/umc_v12_0.c|  60 -
 drivers/gpu/drm/amd/amdgpu/umc_v12_0.h|   3 +
 .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  |   6 +-
 7 files changed, 142 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
index 59fafb8392e0ba..666fd8fa39ad5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
@@ -256,9 +256,14 @@ int amdgpu_mca_smu_log_ras_error(struct amdgpu_device 
*adev, enum amdgpu_ras_blo
if (type == AMDGPU_MCA_ERROR_TYPE_UE)
amdgpu_ras_error_statistic_ue_count(err_data,
_info, _addr, (uint64_t)count);
-   else
-   amdgpu_ras_error_statistic_ce_count(err_data,
-   _info, _addr, (uint64_t)count);
+   else {
+   if 
(!!(MCA_REG__STATUS__DEFERRED(entry->regs[MCA_REG_IDX_STATUS])))
+   amdgpu_ras_error_statistic_de_count(err_data,
+   _info, _addr, (uint64_t)count);
+   else
+   amdgpu_ras_error_statistic_ce_count(err_data,
+   _info, _addr, (uint64_t)count);
+   }
}
 
 out_mca_release:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index ded19182dc792a..94ba10b4184349 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1036,7 +1036,8 @@ static void amdgpu_ras_error_print_error_data(struct 
amdgpu_device *adev,
  struct ras_manager *ras_mgr,
  struct ras_err_data *err_data,
  const char *blk_name,
- bool is_ue)
+ bool is_ue,
+ bool is_de)
 {
struct amdgpu_smuio_mcm_config_info *mcm_info;
struct ras_err_node *err_node;
@@ -1065,25 +1066,50 @@ static void amdgpu_ras_error_print_error_data(struct 
amdgpu_device *adev,
}
 
} else {
-   for_each_ras_error(err_node, err_data) {
-   err_info = _node->err_info;
-   mcm_info = _info->mcm_info;
-   if (err_info->ce_count) {
+   if (is_de) {
+   for_each_ras_error(err_node, err_data) {
+   err_info = _node->err_info;
+   mcm_info = _info->mcm_info;
+   if (err_info->de_count) {
+   dev_info(adev->dev, "socket: %d, die: 
%d, "
+   "%lld new deferred hardware 
errors detected in %s block\n",
+   mcm_info->socket_id,
+   mcm_info->die_id,
+   err_info->de_count,
+   blk_name);
+   }
+   }
+
+   for_each_ras_error(err_node, _mgr->err_data) {
+   err_info = _node->err_info;
+   mcm_info = _info->mcm_info;
dev_info(adev->dev, "socket: %d, die: %d, "
-"%lld new correctable hardware errors 
detected in %s block\n",
-mcm_info->socket_id,
-mcm_info->die_id,
-err_info->ce_count,
-blk_name);
+   "%lld deferred hardware errors detected 
in total in %s block\n",
+   mcm_info->socket_id, mcm_info->die_id,
+   err_info->de_count, blk_name);
+   }
+   } else {
+   for_each_ras_error(err_node, err_data) {
+   err_info = _node->err_info;
+   mcm_info = _info->mcm_info;
+   if (err_info->ce_count) {
+   dev_info(adev->dev, "socket: %d, die: 
%d, "
+   "%lld new 

[PATCH] drm/amdkfd: Fix 'node' NULL check in 'svm_range_get_range_boundaries()'

[Srinivasan Shanmugam]

Range interval [start, last] is ordered by rb_tree, rb_prev, rb_next
return value still needs NULL check, thus modified from "node" to "rb_node".

Fixes the below:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c:2691 
svm_range_get_range_boundaries() warn: can 'node' even be NULL?

Suggested-by: Philip Yang 
Cc: Felix Kuehling 
Cc: Christian König 
Cc: Alex Deucher 
Signed-off-by: Srinivasan Shanmugam 
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 3b78e48832e9..6aa032731ddc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2656,6 +2656,7 @@ svm_range_get_range_boundaries(struct kfd_process *p, 
int64_t addr,
 {
struct vm_area_struct *vma;
struct interval_tree_node *node;
+   struct rb_node *rb_node;
unsigned long start_limit, end_limit;
 
vma = vma_lookup(p->mm, addr << PAGE_SHIFT);
@@ -2678,16 +2679,15 @@ svm_range_get_range_boundaries(struct kfd_process *p, 
int64_t addr,
if (node) {
end_limit = min(end_limit, node->start);
/* Last range that ends before the fault address */
-   node = container_of(rb_prev(>rb),
-   struct interval_tree_node, rb);
+   rb_node = rb_prev(>rb);
} else {
/* Last range must end before addr because
 * there was no range after addr
 */
-   node = container_of(rb_last(>svms.objects.rb_root),
-   struct interval_tree_node, rb);
+   rb_node = rb_last(>svms.objects.rb_root);
}
-   if (node) {
+   if (rb_node) {
+   node = container_of(rb_node, struct interval_tree_node, rb);
if (node->last >= addr) {
WARN(1, "Overlap with prev node and page fault addr\n");
return -EFAULT;
-- 
2.34.1

Re: Documentation for RGB strip on RX 7900 XTX (Reference)

[Alexander Koskovich]

Are there any userspace utilities for checking out the ATOMBIOS tables? Have 
never done so and all the utilities I've found online are too old for this card 
(at least it refuses to open the VBIOS for this card).


On Tuesday, January 9th, 2024 at 3:02 AM, Christian König 
 wrote:


> 
> 
> Am 08.01.24 um 23:32 schrieb Deucher, Alexander:
> 
> > [Public]
> > 
> > > -Original Message-
> > > From: amd-gfx amd-gfx-boun...@lists.freedesktop.org On Behalf Of
> > > Alexander Koskovich
> > > Sent: Sunday, January 7, 2024 11:19 PM
> > > To: amd-gfx@lists.freedesktop.org
> > > Subject: Documentation for RGB strip on RX 7900 XTX (Reference)
> > > 
> > > Hello,
> > > 
> > > I was wondering if AMD would be able provide any documentation for the
> > > RGB strip on the reference cooler
> > > (https://www.amd.com/en/products/graphics/amd-radeon-rx-7900xtx)? It
> > > looks to be handled via I2C commands to the SMU, but having proper
> > > documentation would be extremely helpful.
> > > It depends on the AIB/OEM and how they designed the specific board. The 
> > > RGB controller will either be attached to the DDCVGA i2c bus on the 
> > > display hardware or the second SMU i2c bus. The former will require 
> > > changes to the amdgpu display code to register display i2c buses that are 
> > > not used by the display connectors on the board so they can be used by 
> > > 3rd party applications. Currently we only register i2c buses used for 
> > > display connectors. The latter buses are already registered with the i2c 
> > > subsystem since they are used for other things like EEPROMs on server and 
> > > workstation cards and should be available via standard Linux i2c APIs. 
> > > I'm not sure what i2c LED controllers each AIB vendor uses off hand. 
> > > https://openrgb.org/index.html would probably be a good resource for that 
> > > information.
> 
> 
> 
> It might also be a good idea to look some of the ATOMBIOS tables found
> on your device.
> 
> Those tables are filled in by the AIB/OEM with the information which
> connectors (HDMI, DVI, DP etc...) are on the board and I bet that the
> information which RGB controller is used and where to find it is
> somewhere in there as well.
> 
> Adding Harry from our display team, might be that he has some more hints
> as well.
> 
> Christian.
> 
> > Alex

Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace

[Andri Yngvason]

Hi Daniel,

þri., 9. jan. 2024 kl. 22:32 skrifaði Daniel Stone :

> On Tue, 9 Jan 2024 at 18:12, Andri Yngvason  wrote:
> > + * active color format:
> > + * This read-only property tells userspace the color format
> 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. Drivers shall use
> > + * drm_connector_attach_active_color_format_property() to install
> this
> > + * property. Possible values are "not applicable", "rgb",
> "ycbcr444",
> > + * "ycbcr422", and "ycbcr420".
>
> How does userspace determine what's happened without polling? Will it
> only change after an `ALLOW_MODESET` commit, and be guaranteed to be
> updated after the commit has completed and the event being sent?
> Should it send a HOTPLUG event? Other?
>

Userspace does not determine what's happened without polling. The purpose
of this property is not for programmatic verification that the preferred
property was applied. It is my understanding that it's mostly intended for
debugging purposes. It should only change as a consequence of modesetting,
although I didn't actually look into what happens if you set the "preferred
color format" outside of a modeset.

The way I've implemented things in sway, calling the
"preferred_signal_format" command triggers a modeset with the "preferred
color format" set and calling "get_outputs", immediately queries the
"actual color format" and displays it.

Regards,
Andri

RE: Documentation for RGB strip on RX 7900 XTX (Reference)

[Alexander Koskovich]

Is the AIB/OEM for this board not AMD?
https://www.amd.com/en/products/graphics/amd-radeon-rx-7900xtx



On Tuesday, January 9th, 2024 at 4:53 PM, Deucher, Alexander 
 wrote:


> 
> 
> [AMD Official Use Only - General]
> 
> > -Original Message-
> > From: Alexander Koskovich akoskov...@protonmail.com
> > Sent: Tuesday, January 9, 2024 3:27 PM
> > To: Deucher, Alexander alexander.deuc...@amd.com
> > Cc: amd-gfx@lists.freedesktop.org
> > Subject: RE: Documentation for RGB strip on RX 7900 XTX (Reference)
> > 
> > Doe AMD have documentation on the i2c data that gets sent currently
> > though? I was hoping to figure out what you need to change in the command
> > that gets sent to change stuff like brightness, color (red, green, blue), 
> > rainbow,
> > morse code, etc.
> 
> 
> It depends on the LED controller used by the AIB/OEM. The programming 
> sequence is dependent on the LED controller.
> 
> Alex
> 
> > On Tuesday, January 9th, 2024 at 10:10 AM, Deucher, Alexander
> > alexander.deuc...@amd.com wrote:
> > 
> > > [Public]
> > > 
> > > > -Original Message-
> > > > From: Alexander Koskovich akoskov...@protonmail.com
> > > > Sent: Monday, January 8, 2024 7:22 PM
> > > > To: Deucher, Alexander alexander.deuc...@amd.com
> > > > Cc: amd-gfx@lists.freedesktop.org
> > > > Subject: RE: Documentation for RGB strip on RX 7900 XTX (Reference)
> > > > 
> > > > Currently the reference cooler from AMD does not have an existing
> > > > RGB controller for OpenRGB, that's why I was looking for
> > > > documentation on the I2C commands to send to the second SMU, so I
> > > > don't risk bricking my card by sending wrong commands during
> > > > development somehow.
> > > > 
> > > > writeSetCMDWithData:
> > > > **
> > > > adli2c.iSize = sizeof(ADLI2C)
> > > > adli2c.iAction = ADL_DL_I2C_ACTIONWRITE adli2c.iAddress = 0xb4
> > > > adli2c.iSpeed = 100
> > > > 0 --
> > > > Dev 0: ADL_Display_WriteAndReadSMUI2C(0, ) = 0
> > > > adli2c.iDataSize =
> > > > 24 i2cData[0]~[24]
> > > > 40 51 2c 01 00 00 ff 00 ff ff ff cc 00 cc 00 00 00 ff ff ff ff ff ff
> > > > ff
> > > > 
> > > > From the RGB app's logs this is an example of what the official AMD
> > > > application on Windows is sending when it changes colors on the RGB 
> > > > strip.
> > > > 
> > > > From this can it be assumed the AMD card is using the latter method
> > > > you mentioned with the second SMU I2C bus, in which case no driver
> > > > changes would be needed?
> > > 
> > > IIRC, each AIB/OEM uses its own preferred RGB controller. The reference
> > > board just defines which i2c buses can be used. The RGB control 
> > > application is
> > > just a userspace app provided by the AIB/OEM that calls ADL to talk to
> > > whichever i2c bus the vendor put their RGB controller on. On Linux you 
> > > can do
> > > something similar using the i2c_dev module to open a connection to the i2c
> > > bus driver provided by the kernel. I believe that is what openRGB does 
> > > today.
> > > It looks like you already have the programming sequence above.
> > > 
> > > Alex
> > > 
> > > > On Monday, January 8th, 2024 at 5:32 PM, Deucher, Alexander
> > > > alexander.deuc...@amd.com wrote:
> > > > 
> > > > > [Public]
> > > > > 
> > > > > > -Original Message-
> > > > > > From: amd-gfx amd-gfx-boun...@lists.freedesktop.org On Behalf Of
> > > > > > Alexander Koskovich
> > > > > > Sent: Sunday, January 7, 2024 11:19 PM
> > > > > > To: amd-gfx@lists.freedesktop.org
> > > > > > Subject: Documentation for RGB strip on RX 7900 XTX (Reference)
> > > > > > 
> > > > > > Hello,
> > > > > > 
> > > > > > I was wondering if AMD would be able provide any documentation
> > > > > > for the RGB strip on the reference cooler
> > > > > > (https://www.amd.com/en/products/graphics/amd-radeon-rx-
> > > > > > 7900xtx)?
> > > > > > It
> > > > > > looks to be handled via I2C commands to the SMU, but having
> > > > > > proper documentation would be extremely helpful.
> > > > > 
> > > > > It depends on the AIB/OEM and how they designed the specific
> > > > > board. The RGB controller will either be attached to the DDCVGA
> > > > > i2c bus on the display hardware or the second SMU i2c bus. The
> > > > > former will require changes to the amdgpu display code to register
> > > > > display i2c buses that are not used by the display connectors on the
> > > > > board so they can be used by 3rd party applications.
> > > > > Currently we only register i2c buses used for display connectors.
> > > > > The latter buses are already registered with the i2c subsystem
> > > > > since they are used for other things like EEPROMs on server and
> > > > > workstation cards and should be available via standard Linux i2c
> > > > > APIs. I'm not sure what i2c LED controllers each AIB vendor uses
> > > > > off hand. https://openrgb.org/index.html would probably be a good
> > > > > resource for that information.
> > > 

回复: Re: 回复: Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2

[李真能]

My plan is to obtain the process priority  and convert it into the drm-scheduler's priority, so that the user is unaware and does not need to use environment variables to set it.
This is my patch:--- a/amdgpu/amdgpu_cs.c+++ b/amdgpu/amdgpu_cs.c@@ -31,6 +31,7 @@#if HAVE_ALLOCA_H# include #endif+#include #include "xf86drm.h"#include "amdgpu_drm.h"@@ -72,6 +73,14 @@ drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,}}+ int nice = getpriority(PRIO_PROCESS, 0);+ if (errno != EPERM)+ {+ if (nice > 0)+ priority = AMDGPU_CTX_PRIORITY_LOW;+ else if (nice < 0)+ priority = AMDGPU_CTX_PRIORITY_HIGH;+ }gpu_context = calloc(1, sizeof(struct amdgpu_context));if (!gpu_context)return -ENOMEM;

 

主　题：Re: 回复: Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2 日　期：2024-01-09 17:36 发件人：maraeo 收件人：Christian König;




int p = -1.
unsigned u = p;
int p2 = u;
 
p2 is -1.
 
Marek



On Tue, Jan 9, 2024, 03:26 Christian König  wrote:

Am 09.01.24 um 09:09 schrieb 李真能:
Thanks!
What about the second patch?
The second patch:   amdgpu: change proirity value to be consistent with kernel.
As I want to pass AMDGPU_CTX_PRIORITY_LOW to kernel module drm-scheduler, if these two patches are not applyed, 
It can not pass LOW priority to drm-scheduler.
Do you have any other suggestion?
Well what exactly is the problem? Just use AMD_PRIORITY=-512.As far as I can see that is how it is supposed to be used.Regards,Christian.

 

主　题：Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2 日　期：2024-01-09 15:15 发件人：Christian König 收件人：李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;



Am 09.01.24 um 02:50 schrieb 李真能:
When the priority value is passed to the kernel, the kernel compares it with the following values:
#define AMDGPU_CTX_PRIORITY_VERY_LOW    -1023#define AMDGPU_CTX_PRIORITY_LOW -512#define AMDGPU_CTX_PRIORITY_NORMAL  0#define AMDGPU_CTX_PRIORITY_HIGH    512#define AMDGPU_CTX_PRIORITY_VERY_HIGH   1023
If priority is uint32_t, we can't set LOW and VERY_LOW value to kernel context priority,
Well that's nonsense.How the kernel handles the values and how userspace handles them are two separate things. You just need to make sure that it's always 32 bits.In other words if you have signed or unsigned data type in userspace is irrelevant for the kernel.
You can refer to the kernel function amdgpu_ctx_priority_permit, if priority is greater
than 0, and this process has not  CAP_SYS_NICE capibility or DRM_MASTER permission,
this process will be exited.
Correct, that's intentional.Regards,Christian.

 

主　题：Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2 日　期：2024-01-09 00:28 发件人：Christian König 收件人：李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;



Am 08.01.24 um 10:40 schrieb Zhenneng Li:> In order to pass the correct priority parameter to the kernel,> we must change priority type from uint32_t to int32_t.Hui what? Why should it matter if the parameter is signed or not?That doesn't seem to make sense.Regards,Christian.>> Signed-off-by: Zhenneng Li > ---> amdgpu/amdgpu.h | 2 +-> amdgpu/amdgpu_cs.c | 2 +-> 2 files changed, 2 insertions(+), 2 deletions(-)>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h> index 9bdbf366..f46753f3 100644> --- a/amdgpu/amdgpu.h> +++ b/amdgpu/amdgpu.h> @@ -896,7 +896,7 @@ int amdgpu_bo_list_update(amdgpu_bo_list_handle handle,> *> */> int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,> - uint32_t priority,> + int32_t priority,> amdgpu_context_handle *context);> /**> * Create GPU execution Context> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c> index 49fc16c3..eb72c638 100644> --- a/amdgpu/amdgpu_cs.c> +++ b/amdgpu/amdgpu_cs.c> @@ -49,7 +49,7 @@ static int amdgpu_cs_reset_sem(amdgpu_semaphore_handle sem);> * \return 0 on success otherwise POSIX Error code> */> drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,> - uint32_t priority,> + int32_t priority,> amdgpu_context_handle *context)> {> struct amdgpu_context *gpu_context;

RE: Documentation for RGB strip on RX 7900 XTX (Reference)

[Alexander Koskovich]

I initially tried reaching out to the Coolermaster technical email, but I got 
no response. Is there a better contact for something like this?



On Tuesday, January 9th, 2024 at 5:58 PM, Deucher, Alexander 
 wrote:


> 
> 
> [Public]
> 
> > -Original Message-
> > From: amd-gfx amd-gfx-boun...@lists.freedesktop.org On Behalf Of
> > Deucher, Alexander
> > Sent: Tuesday, January 9, 2024 5:29 PM
> > To: Alexander Koskovich akoskov...@protonmail.com
> > Cc: amd-gfx@lists.freedesktop.org
> > Subject: RE: Documentation for RGB strip on RX 7900 XTX (Reference)
> > 
> > > -Original Message-
> > > From: Alexander Koskovich akoskov...@protonmail.com
> > > Sent: Tuesday, January 9, 2024 4:59 PM
> > > To: Deucher, Alexander alexander.deuc...@amd.com
> > > Cc: amd-gfx@lists.freedesktop.org
> > > Subject: RE: Documentation for RGB strip on RX 7900 XTX (Reference)
> > > 
> > > Is the AIB/OEM for this board not AMD?
> > > https://www.amd.com/en/products/graphics/amd-radeon-rx-7900xtx
> > 
> > I'll double check (we usually don't produce reference boards with RGB), but
> > my understanding is that if any of the boards available for sale on amd.com
> > has RGB controls, the RGB control is provided by a third party vendor.
> 
> 
> 
> CoolerMaster provides the RGB solution. See:
> https://www.amd.com/en/support/graphics/amd-radeon-rx-7000-series/amd-radeon-rx-7900-series/amd-radeon-rx-7900xtx
> 
> Alex
> 
> > Alex
> > 
> > > On Tuesday, January 9th, 2024 at 4:53 PM, Deucher, Alexander
> > > alexander.deuc...@amd.com wrote:
> > > 
> > > > [AMD Official Use Only - General]
> > > > 
> > > > > -Original Message-
> > > > > From: Alexander Koskovich akoskov...@protonmail.com
> > > > > Sent: Tuesday, January 9, 2024 3:27 PM
> > > > > To: Deucher, Alexander alexander.deuc...@amd.com
> > > > > Cc: amd-gfx@lists.freedesktop.org
> > > > > Subject: RE: Documentation for RGB strip on RX 7900 XTX
> > > > > (Reference)
> > > > > 
> > > > > Doe AMD have documentation on the i2c data that gets sent
> > > > > currently though? I was hoping to figure out what you need to
> > > > > change in the command that gets sent to change stuff like
> > > > > brightness, color (red, green, blue), rainbow, morse code, etc.
> > > > 
> > > > It depends on the LED controller used by the AIB/OEM. The
> > > > programming
> > > > sequence is dependent on the LED controller.
> > > > 
> > > > Alex
> > > > 
> > > > > On Tuesday, January 9th, 2024 at 10:10 AM, Deucher, Alexander
> > > > > alexander.deuc...@amd.com wrote:
> > > > > 
> > > > > > [Public]
> > > > > > 
> > > > > > > -Original Message-
> > > > > > > From: Alexander Koskovich akoskov...@protonmail.com
> > > > > > > Sent: Monday, January 8, 2024 7:22 PM
> > > > > > > To: Deucher, Alexander alexander.deuc...@amd.com
> > > > > > > Cc: amd-gfx@lists.freedesktop.org
> > > > > > > Subject: RE: Documentation for RGB strip on RX 7900 XTX
> > > > > > > (Reference)
> > > > > > > 
> > > > > > > Currently the reference cooler from AMD does not have an
> > > > > > > existing RGB controller for OpenRGB, that's why I was looking
> > > > > > > for documentation on the I2C commands to send to the second
> > > > > > > SMU, so I don't risk bricking my card by sending wrong
> > > > > > > commands during development somehow.
> > > > > > > 
> > > > > > > writeSetCMDWithData:
> > > 
> > > **
> > > 
> > > > > > > adli2c.iSize = sizeof(ADLI2C)
> > > > > > > adli2c.iAction = ADL_DL_I2C_ACTIONWRITE adli2c.iAddress = 0xb4
> > > > > > > adli2c.iSpeed = 100
> > > > > > > 0 --
> > > > > > > Dev 0: ADL_Display_WriteAndReadSMUI2C(0, ) = 0
> > > > > > > adli2c.iDataSize =
> > > > > > > 24 i2cData[0]~[24]
> > > > > > > 40 51 2c 01 00 00 ff 00 ff ff ff cc 00 cc 00 00 00 ff ff ff ff
> > > > > > > ff ff ff
> > > > > > > 
> > > > > > > From the RGB app's logs this is an example of what the
> > > > > > > official AMD application on Windows is sending when it changes
> > > > > > > colors on the
> > > > > > > RGB strip.
> > > > > > > 
> > > > > > > From this can it be assumed the AMD card is using the latter
> > > > > > > method you mentioned with the second SMU I2C bus, in which
> > > > > > > case no driver changes would be needed?
> > > > > > 
> > > > > > IIRC, each AIB/OEM uses its own preferred RGB controller. The
> > > > > > reference board just defines which i2c buses can be used. The
> > > > > > RGB control application is just a userspace app provided by the
> > > > > > AIB/OEM that calls ADL to talk to whichever i2c bus the vendor
> > > > > > put their RGB controller on. On Linux you can do something
> > > > > > similar using the i2c_dev module to open a connection to the i2c
> > > > > > bus driver
> > > > > > provided by the kernel. I believe that is what openRGB does today.
> > > > > > It looks like you already have the programming sequence above.
> > > > > > 
> > > > > > Alex
> > > > > > 
> > > > > 

Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace

[Andri Yngvason]

Hi Daniel,

Please excuse my misconfigured email client. HTML was accidentally
enabled in my previous messages, so I'll re-send it for the benefit of
mailing lists.

þri., 9. jan. 2024 kl. 22:32 skrifaði Daniel Stone :
>
> On Tue, 9 Jan 2024 at 18:12, Andri Yngvason  wrote:
> > + * active color format:
> > + * This read-only property tells userspace the color format 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. Drivers shall use
> > + * drm_connector_attach_active_color_format_property() to install this
> > + * property. Possible values are "not applicable", "rgb", "ycbcr444",
> > + * "ycbcr422", and "ycbcr420".
>
> How does userspace determine what's happened without polling? Will it
> only change after an `ALLOW_MODESET` commit, and be guaranteed to be
> updated after the commit has completed and the event being sent?
> Should it send a HOTPLUG event? Other?

Userspace does not determine what's happened without polling. The
purpose of this property is not for programmatic verification that the
preferred property was applied. It is my understanding that it's
mostly intended for debugging purposes. It should only change as a
consequence of modesetting, although I didn't actually look into what
happens if you set the "preferred color format" outside of a modeset.

The way I've implemented things in sway, calling the
"preferred_signal_format" command triggers a modeset with the
"preferred color format" set and calling "get_outputs", immediately
queries the "actual color format" and displays it.

Regards,
Andri

RE: Documentation for RGB strip on RX 7900 XTX (Reference)

[Alexander Koskovich]

Doe AMD have documentation on the i2c data that gets sent currently though? I 
was hoping to figure out what you need to change in the command that gets sent 
to change stuff like brightness, color (red, green, blue), rainbow, morse code, 
etc.

On Tuesday, January 9th, 2024 at 10:10 AM, Deucher, Alexander 
 wrote:


> 
> 
> [Public]
> 
> > -Original Message-
> > From: Alexander Koskovich akoskov...@protonmail.com
> > Sent: Monday, January 8, 2024 7:22 PM
> > To: Deucher, Alexander alexander.deuc...@amd.com
> > Cc: amd-gfx@lists.freedesktop.org
> > Subject: RE: Documentation for RGB strip on RX 7900 XTX (Reference)
> > 
> > Currently the reference cooler from AMD does not have an existing RGB
> > controller for OpenRGB, that's why I was looking for documentation on the
> > I2C commands to send to the second SMU, so I don't risk bricking my card by
> > sending wrong commands during development somehow.
> > 
> > writeSetCMDWithData:
> > **
> > adli2c.iSize = sizeof(ADLI2C)
> > adli2c.iAction = ADL_DL_I2C_ACTIONWRITE
> > adli2c.iAddress = 0xb4
> > adli2c.iSpeed = 100
> > 0 --
> > Dev 0: ADL_Display_WriteAndReadSMUI2C(0, ) = 0 adli2c.iDataSize =
> > 24 i2cData[0]~[24]
> > 40 51 2c 01 00 00 ff 00 ff ff ff cc 00 cc 00 00 00 ff ff ff ff ff ff ff
> > 
> > From the RGB app's logs this is an example of what the official AMD
> > application on Windows is sending when it changes colors on the RGB strip.
> > 
> > From this can it be assumed the AMD card is using the latter method you
> > mentioned with the second SMU I2C bus, in which case no driver changes
> > would be needed?
> 
> 
> 
> IIRC, each AIB/OEM uses its own preferred RGB controller. The reference board 
> just defines which i2c buses can be used. The RGB control application is just 
> a userspace app provided by the AIB/OEM that calls ADL to talk to whichever 
> i2c bus the vendor put their RGB controller on. On Linux you can do something 
> similar using the i2c_dev module to open a connection to the i2c bus driver 
> provided by the kernel. I believe that is what openRGB does today. It looks 
> like you already have the programming sequence above.
> 
> Alex
> 
> > On Monday, January 8th, 2024 at 5:32 PM, Deucher, Alexander
> > alexander.deuc...@amd.com wrote:
> > 
> > > [Public]
> > > 
> > > > -Original Message-
> > > > From: amd-gfx amd-gfx-boun...@lists.freedesktop.org On Behalf Of
> > > > Alexander Koskovich
> > > > Sent: Sunday, January 7, 2024 11:19 PM
> > > > To: amd-gfx@lists.freedesktop.org
> > > > Subject: Documentation for RGB strip on RX 7900 XTX (Reference)
> > > > 
> > > > Hello,
> > > > 
> > > > I was wondering if AMD would be able provide any documentation for
> > > > the RGB strip on the reference cooler
> > > > (https://www.amd.com/en/products/graphics/amd-radeon-rx-7900xtx)?
> > > > It
> > > > looks to be handled via I2C commands to the SMU, but having proper
> > > > documentation would be extremely helpful.
> > > 
> > > It depends on the AIB/OEM and how they designed the specific board. The
> > > RGB controller will either be attached to the DDCVGA i2c bus on the 
> > > display
> > > hardware or the second SMU i2c bus. The former will require changes to the
> > > amdgpu display code to register display i2c buses that are not used by the
> > > display connectors on the board so they can be used by 3rd party 
> > > applications.
> > > Currently we only register i2c buses used for display connectors. The 
> > > latter
> > > buses are already registered with the i2c subsystem since they are used 
> > > for
> > > other things like EEPROMs on server and workstation cards and should be
> > > available via standard Linux i2c APIs. I'm not sure what i2c LED 
> > > controllers each
> > > AIB vendor uses off hand. https://openrgb.org/index.html would probably be
> > > a good resource for that information.
> > > 
> > > Alex