Re: [PATCH V2 2/7] drm/amd/pm: drop the pptable related workarounds for SMU 13.0.0

2022-09-19 Thread Lazar, Lijo




On 9/19/2022 7:32 AM, Evan Quan wrote:

The pptable in the vbios is fully ready. The related workarounds
in driver are not needed any more.

Signed-off-by: Evan Quan 


Reviewed-by: Lijo Lazar 

Thanks,
Lijo


Change-Id: I2549cd1acd6eebde361ed8e27d5631bd57644e52
--
v1->v2:
   - drop unrelated and unnecessary changes(Alex, Guchun)
---
  .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 46 ++--
  .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c  | 54 ++-
  2 files changed, 6 insertions(+), 94 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index 4fd685af8fa4..53d26bca524a 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
@@ -210,7 +210,8 @@ int smu_v13_0_init_pptable_microcode(struct smu_context 
*smu)
if (!adev->scpm_enabled)
return 0;
  
-	if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 7))

+   if ((adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 7)) ||
+   (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 0)))
return 0;
  
  	/* override pptable_id from driver parameter */

@@ -219,27 +220,6 @@ int smu_v13_0_init_pptable_microcode(struct smu_context 
*smu)
dev_info(adev->dev, "override pptable id %d\n", pptable_id);
} else {
pptable_id = smu->smu_table.boot_values.pp_table_id;
-
-   /*
-* Temporary solution for SMU V13.0.0 with SCPM enabled:
-*   - use vbios carried pptable when pptable_id is 3664, 3715 
or 3795
-*   - use 36831 soft pptable when pptable_id is 3683
-*/
-   if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 0)) {
-   switch (pptable_id) {
-   case 3664:
-   case 3715:
-   case 3795:
-   pptable_id = 0;
-   break;
-   case 3683:
-   pptable_id = 36831;
-   break;
-   default:
-   dev_err(adev->dev, "Unsupported pptable id 
%d\n", pptable_id);
-   return -EINVAL;
-   }
-   }
}
  
  	/* "pptable_id == 0" means vbios carries the pptable. */

@@ -475,28 +455,8 @@ int smu_v13_0_setup_pptable(struct smu_context *smu)
} else {
pptable_id = smu->smu_table.boot_values.pp_table_id;
  
-		/*

-* Temporary solution for SMU V13.0.0 with SCPM disabled:
-*   - use 3664, 3683 or 3715 on request
-*   - use 3664 when pptable_id is 0
-* TODO: drop these when the pptable carried in vbios is ready.
-*/
-   if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 0)) {
-   switch (pptable_id) {
-   case 0:
-   pptable_id = 3664;
-   break;
-   case 3664:
-   case 3683:
-   case 3715:
-   break;
-   default:
-   dev_err(adev->dev, "Unsupported pptable id 
%d\n", pptable_id);
-   return -EINVAL;
-   }
-   } else if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 
10)) {
+   if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 10))
pptable_id = ;
-   }
}
  
  	/* force using vbios pptable in sriov mode */

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
index a6b7319fbfe6..1d454485e0d9 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
@@ -375,59 +375,11 @@ static int smu_v13_0_0_setup_pptable(struct smu_context 
*smu)
  {
struct smu_table_context *smu_table = >smu_table;
struct amdgpu_device *adev = smu->adev;
-   uint32_t pptable_id;
int ret = 0;
  
-	/*

-* With SCPM enabled, the pptable used will be signed. It cannot
-* be used directly by driver. To get the raw pptable, we need to
-* rely on the combo pptable(and its revelant SMU message).
-*/
-   if (adev->scpm_enabled) {
-   ret = smu_v13_0_0_get_pptable_from_pmfw(smu,
-   
_table->power_play_table,
-   
_table->power_play_table_size);
-   } else {
-   /* override pptable_id from driver parameter */
-   if (amdgpu_smu_pptable_id >= 0) {
-   

Re: [PATCH V2 1/7] drm/amd/pm: add support for 3794 pptable for SMU13.0.0

2022-09-19 Thread Lazar, Lijo




On 9/19/2022 7:32 AM, Evan Quan wrote:

Enable 3794 pptable support for SMU13.0.0.

Signed-off-by: Evan Quan 


Reviewed-by: Lijo Lazar 

Thanks,
Lijo


Change-Id: Ia208531c9eb96611b6136975bcbd8d27007b9e14
---
  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
index de779f8089d3..a6b7319fbfe6 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
@@ -406,6 +406,7 @@ static int smu_v13_0_0_setup_pptable(struct smu_context 
*smu)
case 3664:
case 3715:
case 3795:
+   case 3794:
pptable_id = 0;
break;
case 3683:



Re: [PATCH v3] drm/sched: Add FIFO sched policy to run queue v3

2022-09-19 Thread Luben Tuikov
Please run this patch through checkpatch.pl, as it shows
12 warnings with it. Use these command line options:
"--strict --show-types".

Inlined:

On 2022-09-13 16:40, Andrey Grodzovsky wrote:
> Given many entities competing for same run queue on
> the same scheduler and unacceptably long wait time for some
> jobs waiting stuck in the run queue before being picked up are
> observed (seen using  GPUVis).

Since the second part of this sentence is the result of the first,
I'd say something like "When many entities ... we see unacceptably long ...".

> The issue is due to the Round Robin policy used by schedulers
> to pick up the next entity's job queue for execution. Under stress
> of many entities and long job queus within entity some

Spelling: "queues".

> jobs could be stack for very long time in it's entity's

"stuck", not "stack".

> queue before being popped from the queue and executed
> while for other entities with smaller job queues a job
> might execute earlier even though that job arrived later
> then the job in the long queue.

"than".

>    
> Fix:
> Add FIFO selection policy to entities in run queue, chose next entity
> on run queue in such order that if job on one entity arrived
> earlier then job on another entity the first job will start
> executing earlier regardless of the length of the entity's job
> queue.
>    
> v2:
> Switch to rb tree structure for entities based on TS of
> oldest job waiting in the job queue of an entity. Improves next
> entity extraction to O(1). Entity TS update
> O(log N) where N is the number of entities in the run-queue
>    
> Drop default option in module control parameter.
> 
> v3:
> Various cosmetical fixes and minor refactoring of fifo update function.
> Signed-off-by: Andrey Grodzovsky 
> Tested-by: Li Yunxiang (Teddy) 
> ---
>  drivers/gpu/drm/scheduler/sched_entity.c |  26 -
>  drivers/gpu/drm/scheduler/sched_main.c   | 132 ++-
>  include/drm/gpu_scheduler.h  |  35 ++
>  3 files changed, 187 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
> b/drivers/gpu/drm/scheduler/sched_entity.c
> index 6b25b2f4f5a3..f3ffce3c9304 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -73,6 +73,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
>   entity->priority = priority;
>   entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
>   entity->last_scheduled = NULL;
> + RB_CLEAR_NODE(>rb_tree_node);
>  
>   if(num_sched_list)
>   entity->rq = _list[0]->sched_rq[entity->priority];
> @@ -417,14 +418,16 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct 
> drm_sched_entity *entity)
>  
>   sched_job = to_drm_sched_job(spsc_queue_peek(>job_queue));
>   if (!sched_job)
> - return NULL;
> + goto skip;
>  
>   while ((entity->dependency =
>   drm_sched_job_dependency(sched_job, entity))) {
>   trace_drm_sched_job_wait_dep(sched_job, entity->dependency);
>  
> - if (drm_sched_entity_add_dependency_cb(entity))
> - return NULL;
> + if (drm_sched_entity_add_dependency_cb(entity)) {
> + sched_job = NULL;
> + goto skip;
> + }
>   }
>  
>   /* skip jobs from entity that marked guilty */
> @@ -443,6 +446,16 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct 
> drm_sched_entity *entity)
>   smp_wmb();
>  
>   spsc_queue_pop(>job_queue);
> +
> + /*
> +  * It's when head job is extracted we can access the next job (or empty)
> +  * queue and update the entity location in the min heap accordingly.
> +  */
> +skip:
> + if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
> + drm_sched_rq_update_fifo(entity,
> +  (sched_job ? sched_job->submit_ts : 
> ktime_get()));
> +
>   return sched_job;
>  }
>  
> @@ -502,11 +515,13 @@ void drm_sched_entity_push_job(struct drm_sched_job 
> *sched_job)
>  {
>   struct drm_sched_entity *entity = sched_job->entity;
>   bool first;
> + ktime_t ts =  ktime_get();
>  
>   trace_drm_sched_job(sched_job, entity);
>   atomic_inc(entity->rq->sched->score);
>   WRITE_ONCE(entity->last_user, current->group_leader);
>   first = spsc_queue_push(>job_queue, _job->queue_node);
> + sched_job->submit_ts = ts;
>  
>   /* first job wakes up scheduler */
>   if (first) {
> @@ -518,8 +533,13 @@ void drm_sched_entity_push_job(struct drm_sched_job 
> *sched_job)
>   DRM_ERROR("Trying to push to a killed entity\n");
>   return;
>   }
> +
>   drm_sched_rq_add_entity(entity->rq, entity);
>   spin_unlock(>rq_lock);
> +
> + if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
> + 

RE: [v2] drm/amd: Add IMU fw version to fw version queries

2022-09-19 Thread Gao, Likun
[AMD Official Use Only - General]

Reviewed-by: Likun Gao .

Regards,
Likun

-Original Message-
From: Francis, David  
Sent: Monday, September 19, 2022 9:42 PM
To: amd-gfx@lists.freedesktop.org
Cc: Francis, David ; Gao, Likun 
Subject: [v2] drm/amd: Add IMU fw version to fw version queries

IMU is a new firmware for GFX11.

There are four means by which firmware version can be queried from the driver: 
device attributes, vf2pf, debugfs, and the AMDGPU_INFO_FW_VERSION option in the 
amdgpu info ioctl.

Add IMU as an option for those four methods.

V2: Added debugfs

CC: Likun Gao 

Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c   |  4 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c|  1 +
 drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  1 +
 include/uapi/drm/amdgpu_drm.h   |  2 ++
 5 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 1369c25448dc..56753c3574b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -328,6 +328,10 @@ static int amdgpu_firmware_info(struct 
drm_amdgpu_info_firmware *fw_info,
fw_info->ver = adev->psp.cap_fw_version;
fw_info->feature = adev->psp.cap_feature_version;
break;
+   case AMDGPU_INFO_FW_IMU:
+   fw_info->ver = adev->gfx.imu_fw_version;
+   fw_info->feature = 0;
+   break;
default:
return -EINVAL;
}
@@ -1488,6 +1492,15 @@ static int amdgpu_debugfs_firmware_info_show(struct 
seq_file *m, void *unused)
   fw_info.feature, fw_info.ver);
}
 
+   /* IMU */
+   query_fw.fw_type = AMDGPU_INFO_FW_IMU;
+   query_fw.index = 0;
+   ret = amdgpu_firmware_info(_info, _fw, adev);
+   if (ret)
+   return ret;
+   seq_printf(m, "IMU feature version: %u, firmware version: 0x%08x\n",
+  fw_info.feature, fw_info.ver);
+
/* PSP SOS */
query_fw.fw_type = AMDGPU_INFO_FW_SOS;
ret = amdgpu_firmware_info(_info, _fw, adev); diff --git 
a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 939c8614f0e3..a576a50fad25 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -630,6 +630,7 @@ FW_VERSION_ATTR(rlc_srlg_fw_version, 0444, 
gfx.rlc_srlg_fw_version);  FW_VERSION_ATTR(rlc_srls_fw_version, 0444, 
gfx.rlc_srls_fw_version);  FW_VERSION_ATTR(mec_fw_version, 0444, 
gfx.mec_fw_version);  FW_VERSION_ATTR(mec2_fw_version, 0444, 
gfx.mec2_fw_version);
+FW_VERSION_ATTR(imu_fw_version, 0444, gfx.imu_fw_version);
 FW_VERSION_ATTR(sos_fw_version, 0444, psp.sos.fw_version);  
FW_VERSION_ATTR(asd_fw_version, 0444, psp.asd_context.bin_desc.fw_version);
 FW_VERSION_ATTR(ta_ras_fw_version, 0444, 
psp.ras_context.context.bin_desc.fw_version);
@@ -651,7 +652,8 @@ static struct attribute *fw_attrs[] = {
_attr_ta_ras_fw_version.attr, _attr_ta_xgmi_fw_version.attr,
_attr_smc_fw_version.attr, _attr_sdma_fw_version.attr,
_attr_sdma2_fw_version.attr, _attr_vcn_fw_version.attr,
-   _attr_dmcu_fw_version.attr, NULL
+   _attr_dmcu_fw_version.attr, _attr_imu_fw_version.attr,
+   NULL
 };
 
 static const struct attribute_group fw_attr_group = { diff --git 
a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index e4af40b9a8aa..38c46f09d784 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -547,6 +547,7 @@ static void amdgpu_virt_populate_vf2pf_ucode_info(struct 
amdgpu_device *adev)
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_RLC_SRLS, 
adev->gfx.rlc_srls_fw_version);
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_MEC,  
adev->gfx.mec_fw_version);
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_MEC2, 
adev->gfx.mec2_fw_version);
+   POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_IMU,  
adev->gfx.imu_fw_version);
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_SOS,  
adev->psp.sos.fw_version);
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_ASD,
adev->psp.asd_context.bin_desc.fw_version);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h 
b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
index e78e4c27b62a..6c97148ca0ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -70,6 +70,7 @@ enum amd_sriov_ucode_engine_id {
AMD_SRIOV_UCODE_ID_RLC_SRLS,
AMD_SRIOV_UCODE_ID_MEC,
AMD_SRIOV_UCODE_ID_MEC2,
+   AMD_SRIOV_UCODE_ID_IMU,
AMD_SRIOV_UCODE_ID_SOS,
AMD_SRIOV_UCODE_ID_ASD,
AMD_SRIOV_UCODE_ID_TA_RAS,
diff --git 

[PATCH] drm/amdgpu: Fixed ras warning when uninstalling amdgpu

2022-09-19 Thread YiPeng Chai
  For the asic using smu v13_0_2, there is the following
warning when uninstalling amdgpu:
  amdgpu: ras disable gfx failed poison:1 ret:-22.

[Why]:
  For the asic using smu v13_0_2, the psp .suspend and
  mode1reset is called before amdgpu_ras_pre_fini during
  amdgpu uninstall, it has disabled all ras features and
  reset the psp. Since the psp is reset, calling
  amdgpu_ras_disable_all_features in amdgpu_ras_pre_fini
  to disable ras features will fail.

[How]:
  If all ras features are disabled, amdgpu_ras_disable_all_features
  will not be called to disable all ras features again.

Signed-off-by: YiPeng Chai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index e55f106621ef..3deb716710e6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -2720,7 +2720,8 @@ int amdgpu_ras_pre_fini(struct amdgpu_device *adev)
 
 
/* Need disable ras on all IPs here before ip [hw/sw]fini */
-   amdgpu_ras_disable_all_features(adev, 0);
+   if (con->features)
+   amdgpu_ras_disable_all_features(adev, 0);
amdgpu_ras_recovery_fini(adev);
return 0;
 }
-- 
2.25.1



Re: [PATCH 4/4] drm/amdgpu: Implement OS triggered MCBP(v4)

2022-09-19 Thread Luben Tuikov
Inline:

On 2022-09-15 04:16, jiadong@amd.com wrote:
> From: "Jiadong.Zhu" 
> 
> Trigger Mid-Command Buffer Preemption according to the priority of the 
> software
> rings and the hw fence signalling condition.
> 
> The muxer saves the locations of the indirect buffer frames from the software
> ring together with the fence sequence number in its fifo queue, and pops out
> those records when the fences are signalled. The locations are used to 
> resubmit
> packages in preemption scenarios by coping the chunks from the software ring.
> 
> v2: Update comment style.
> v3: Fix conflict caused by previous modifications.
> v4: Remove unnecessary prints.
> 
> Cc: Christian Koenig 
> Cc: Luben Tuikov 
> Cc: Andrey Grodzovsky 
> Signed-off-by: Jiadong.Zhu 
> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile  |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |   2 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mcbp.c |  91 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mcbp.h |  29 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c |  12 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |   3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c | 153 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.h |  22 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.c  |  27 
>  9 files changed, 340 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_mcbp.c
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_mcbp.h
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 85224bc81ce5..24c5aa19bbf2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -59,7 +59,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>   amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
>   amdgpu_fw_attestation.o amdgpu_securedisplay.o \
>   amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \
> - amdgpu_sw_ring.o amdgpu_ring_mux.o
> + amdgpu_sw_ring.o amdgpu_ring_mux.o amdgpu_mcbp.o
>  
>  amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 258cffe3c06a..af86d87e2f3b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -211,6 +211,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
> num_ibs,
>   }
>   }
>  
> + amdgpu_ring_ib_begin(ring);
>   if (job && ring->funcs->init_cond_exec)
>   patch_offset = amdgpu_ring_init_cond_exec(ring);
>  
> @@ -285,6 +286,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
> num_ibs,
>   ring->hw_prio == AMDGPU_GFX_PIPE_PRIO_HIGH)
>   ring->funcs->emit_wave_limit(ring, false);
>  
> + amdgpu_ring_ib_end(ring);
>   amdgpu_ring_commit(ring);
>   return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mcbp.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mcbp.c
> new file mode 100644
> index ..4b0aae1a7ad6
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mcbp.c
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright 2022 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "amdgpu.h"
> +#include "amdgpu_mcbp.h"
> +#include "amdgpu_ring.h"
> +
> +/* trigger mcbp and find if we need resubmit */

This is a new file, new functionality, new capability.
Put a better comment here, not all lower-case, not abbreviated, not missing a 
period at the end.
Change this comment to:

/* Trigger Mid-Command Buffer Preemption (MCBP) and find if we need to 
resubmit. */

With that change, this patch is:

Acked-by: Luben Tuikov 

Christian and Andrey should take a look at this patch as well.


RE: [PATCH V2 2/7] drm/amd/pm: drop the pptable related workarounds for SMU 13.0.0

2022-09-19 Thread Quan, Evan
[AMD Official Use Only - General]

@Deucher, Alexander @Lazar, Lijo Can I have your RB for the first two patches 
of this series? I know there is some arguments for other patches.
As that(missing of the first two patches) is gating the integration of the new 
IFWIs for SMU13.0.0 ASIC.

BR
Evan
> -Original Message-
> From: Quan, Evan 
> Sent: Monday, September 19, 2022 10:03 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Chen, Guchun
> ; Quan, Evan 
> Subject: [PATCH V2 2/7] drm/amd/pm: drop the pptable related
> workarounds for SMU 13.0.0
> 
> The pptable in the vbios is fully ready. The related workarounds in driver are
> not needed any more.
> 
> Signed-off-by: Evan Quan 
> Change-Id: I2549cd1acd6eebde361ed8e27d5631bd57644e52
> --
> v1->v2:
>   - drop unrelated and unnecessary changes(Alex, Guchun)
> ---
>  .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 46 ++--
>  .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c  | 54 ++-
>  2 files changed, 6 insertions(+), 94 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> index 4fd685af8fa4..53d26bca524a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> @@ -210,7 +210,8 @@ int smu_v13_0_init_pptable_microcode(struct
> smu_context *smu)
>   if (!adev->scpm_enabled)
>   return 0;
> 
> - if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 7))
> + if ((adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 7)) ||
> + (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 0)))
>   return 0;
> 
>   /* override pptable_id from driver parameter */ @@ -219,27 +220,6
> @@ int smu_v13_0_init_pptable_microcode(struct smu_context *smu)
>   dev_info(adev->dev, "override pptable id %d\n",
> pptable_id);
>   } else {
>   pptable_id = smu->smu_table.boot_values.pp_table_id;
> -
> - /*
> -  * Temporary solution for SMU V13.0.0 with SCPM enabled:
> -  *   - use vbios carried pptable when pptable_id is 3664, 3715
> or 3795
> -  *   - use 36831 soft pptable when pptable_id is 3683
> -  */
> - if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0,
> 0)) {
> - switch (pptable_id) {
> - case 3664:
> - case 3715:
> - case 3795:
> - pptable_id = 0;
> - break;
> - case 3683:
> - pptable_id = 36831;
> - break;
> - default:
> - dev_err(adev->dev, "Unsupported pptable
> id %d\n", pptable_id);
> - return -EINVAL;
> - }
> - }
>   }
> 
>   /* "pptable_id == 0" means vbios carries the pptable. */ @@ -475,28
> +455,8 @@ int smu_v13_0_setup_pptable(struct smu_context *smu)
>   } else {
>   pptable_id = smu->smu_table.boot_values.pp_table_id;
> 
> - /*
> -  * Temporary solution for SMU V13.0.0 with SCPM disabled:
> -  *   - use 3664, 3683 or 3715 on request
> -  *   - use 3664 when pptable_id is 0
> -  * TODO: drop these when the pptable carried in vbios is
> ready.
> -  */
> - if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0,
> 0)) {
> - switch (pptable_id) {
> - case 0:
> - pptable_id = 3664;
> - break;
> - case 3664:
> - case 3683:
> - case 3715:
> - break;
> - default:
> - dev_err(adev->dev, "Unsupported pptable
> id %d\n", pptable_id);
> - return -EINVAL;
> - }
> - } else if (adev->ip_versions[MP1_HWIP][0] ==
> IP_VERSION(13, 0, 10)) {
> + if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0,
> 10))
>   pptable_id = ;
> - }
>   }
> 
>   /* force using vbios pptable in sriov mode */ diff --git
> a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index a6b7319fbfe6..1d454485e0d9 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -375,59 +375,11 @@ static int smu_v13_0_0_setup_pptable(struct
> smu_context *smu)  {
>   struct smu_table_context *smu_table = >smu_table;
>   struct amdgpu_device *adev = smu->adev;
> - uint32_t pptable_id;
>   int ret = 0;
> 
> - /*
> -  * With SCPM enabled, the pptable used will be signed. 

Re: [PATCH] drm/amdgpu: Introduce gfx software ring(v5)

2022-09-19 Thread Luben Tuikov
Perhaps this patch should be marked with 1/4 and there should be a space
in "ring (v5)".

Also please run your patches through checkpatch.pl with the following options:
"--strict --show-types".

Inlined:

On 2022-09-15 02:48, jiadong@amd.com wrote:
> From: "Jiadong.Zhu" 
> 
> The software ring is created to support priority context while there is only
> one hardware queue for gfx.
> 
> Every software ring has its fence driver and could be used as an ordinary ring
> for the GPU scheduler.
> Multiple software rings are bound to a real ring with the ring muxer. The
> packages committed on the software ring are copied to the real ring.
> 
> v2: Use array to store software ring entry.
> v3: Remove unnecessary prints.
> v4: Remove amdgpu_ring_sw_init/fini functions,
> using gtt for sw ring buffer for later dma copy
> optimization.
> v5: Allocate ring entry dynamicly in the muxer.

"dynamically"

> 
> Cc: Christian Koenig 
> Cc: Luben Tuikov 
> Cc: Andrey Grodzovsky  
> Signed-off-by: Jiadong.Zhu 
> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile  |   3 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h  |   3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |   4 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c | 176 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.h |  66 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.c  |  60 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.h  |  43 +
>  7 files changed, 354 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.h
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.c
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.h
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 3e0e2eb7e235..85224bc81ce5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -58,7 +58,8 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>   amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
>   amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
>   amdgpu_fw_attestation.o amdgpu_securedisplay.o \
> - amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o
> + amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \
> + amdgpu_sw_ring.o amdgpu_ring_mux.o
>  
>  amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 53526ffb2ce1..9996dadb39f7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -33,6 +33,7 @@
>  #include "amdgpu_imu.h"
>  #include "soc15.h"
>  #include "amdgpu_ras.h"
> +#include "amdgpu_ring_mux.h"
>  
>  /* GFX current status */
>  #define AMDGPU_GFX_NORMAL_MODE   0xL
> @@ -346,6 +347,8 @@ struct amdgpu_gfx {
>   struct amdgpu_gfx_ras   *ras;
>  
>   boolis_poweron;
> +
> + struct amdgpu_ring_mux  muxer;
>  };
>  
>  #define amdgpu_gfx_get_gpu_clock_counter(adev) 
> (adev)->gfx.funcs->get_gpu_clock_counter((adev))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 7d89a52091c0..40b1277b4f0c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -278,6 +278,10 @@ struct amdgpu_ring {
>   boolis_mes_queue;
>   uint32_thw_queue_id;
>   struct amdgpu_mes_ctx_data *mes_ctx;
> +
> + boolis_sw_ring;
> + unsigned intentry_index;
> +
>  };
>  
>  #define amdgpu_ring_parse_cs(r, p, job, ib) ((r)->funcs->parse_cs((p), 
> (job), (ib)))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c
> new file mode 100644
> index ..5e9c178f358b
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c
> @@ -0,0 +1,176 @@
> +/*
> + * Copyright 2022 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A 

Re: [BUG][5.20] refcount_t: underflow; use-after-free

2022-09-19 Thread Mikhail Gavrilov
Hi!
Unfortunately the use-after-free issue still happens on the 6.0-rc5 kernel.
The issue became hard to repeat. I spent the whole day at the computer
when use-after-free again happened, I was playing the game Tiny Tina's
Wonderlands.
Therefore, forget about repeatability. It remains only to hope for
logs and tracing.
I didn't see anything new in the logs. It seems that we need to
somehow expand the logging so that the next time this happens we have
more information.

Sep 18 20:52:16 primary-ws gnome-shell[2388]:
meta_window_set_stack_position_no_sync: assertion
'window->stack_position >= 0' failed
Sep 18 20:52:27 primary-ws gnome-shell[2388]:
meta_window_set_stack_position_no_sync: assertion
'window->stack_position >= 0' failed
Sep 18 20:53:44 primary-ws gnome-shell[2388]: Window manager warning:
Window 0x4e3 sets an MWM hint indicating it isn't resizable, but
sets min size 1 x 1 and max size 2147483647 x 2147483647; this doesn't
make much sense.
Sep 18 20:53:45 primary-ws kernel: umip_printk: 11 callbacks suppressed
Sep 18 20:53:45 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:14ebb0d03 sp:4ee528: SGDT instruction cannot be used by
applications.
Sep 18 20:53:45 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:14ebb0d03 sp:4ee528: For now, expensive software emulation returns
the result.
Sep 18 20:53:53 primary-ws gnome-shell[2388]:
meta_window_set_stack_position_no_sync: assertion
'window->stack_position >= 0' failed
Sep 18 20:53:53 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:14ebb0d03 sp:4ee528: SGDT instruction cannot be used by
applications.
Sep 18 20:53:53 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:14ebb0d03 sp:4ee528: For now, expensive software emulation returns
the result.
Sep 18 20:54:15 primary-ws kernel: umip: Wonderlands.exe[214194]
ip:15a270815 sp:6eaef490: SGDT instruction cannot be used by
applications.
Sep 18 20:56:01 primary-ws kernel: umip_printk: 15 callbacks suppressed
Sep 18 20:56:01 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:15e3a82b0 sp:4ed178: SGDT instruction cannot be used by
applications.
Sep 18 20:56:01 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:15e3a82b0 sp:4ed178: For now, expensive software emulation returns
the result.
Sep 18 20:56:03 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:15e3a82b0 sp:4edbe8: SGDT instruction cannot be used by
applications.
Sep 18 20:56:03 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:15e3a82b0 sp:4edbe8: For now, expensive software emulation returns
the result.
Sep 18 20:56:03 primary-ws kernel: umip: Wonderlands.exe[213853]
ip:15e3a82b0 sp:4ebf18: SGDT instruction cannot be used by
applications.
Sep 18 20:57:55 primary-ws kernel: [ cut here ]
Sep 18 20:57:55 primary-ws kernel: refcount_t: underflow; use-after-free.
Sep 18 20:57:55 primary-ws kernel: WARNING: CPU: 22 PID: 235114 at
lib/refcount.c:28 refcount_warn_saturate+0xba/0x110
Sep 18 20:57:55 primary-ws kernel: Modules linked in: tls uinput
rfcomm snd_seq_dummy snd_hrtimer nft_objref nf_conntrack_netbios_ns
nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
nft_reject_inet nf_reject_ipv4 nf_>
Sep 18 20:57:55 primary-ws kernel:  asus_wmi ledtrig_audio
sparse_keymap platform_profile irqbypass rfkill mc rapl snd_timer
video wmi_bmof pcspkr snd k10temp i2c_piix4 soundcore acpi_cpufreq
zram amdgpu drm_ttm_helper ttm iommu_v2 crct1>
Sep 18 20:57:55 primary-ws kernel: Unloaded tainted modules:
amd64_edac():1 amd64_edac():1 amd64_edac():1 amd64_edac():1
amd64_edac():1 amd64_edac():1 amd64_edac():1 amd64_edac():1
amd64_edac():1 pcc_cpufreq():1 pcc_cpufreq():1 amd64_eda>
Sep 18 20:57:55 primary-ws kernel:  pcc_cpufreq():1 pcc_cpufreq():1
fjes():1 fjes():1 pcc_cpufreq():1 fjes():1 fjes():1 fjes():1 fjes():1
fjes():1
Sep 18 20:57:55 primary-ws kernel: CPU: 22 PID: 235114 Comm:
kworker/22:0 Tainted: GWL---  ---
6.0.0-0.rc5.20220914git3245cb65fd91.39.fc38.x86_64 #1
Sep 18 20:57:55 primary-ws kernel: Hardware name: System manufacturer
System Product Name/ROG STRIX X570-I GAMING, BIOS 4403 04/27/2022
Sep 18 20:57:55 primary-ws kernel: Workqueue: events
drm_sched_entity_kill_jobs_work [gpu_sched]
Sep 18 20:57:55 primary-ws kernel: RIP: 0010:refcount_warn_saturate+0xba/0x110
Sep 18 20:57:55 primary-ws kernel: Code: 01 01 e8 69 6b 6f 00 0f 0b e9
32 38 a5 00 80 3d 4d 7d be 01 00 75 85 48 c7 c7 80 b7 8e 95 c6 05 3d
7d be 01 01 e8 46 6b 6f 00 <0f> 0b e9 0f 38 a5 00 80 3d 28 7d be 01 00
0f 85 5e ff ff ff 48 c7
Sep 18 20:57:55 primary-ws kernel: RSP: 0018:a1a853ccbe60 EFLAGS: 00010286
Sep 18 20:57:55 primary-ws kernel: RAX: 0026 RBX:
8e0e60a96c28 RCX: 
Sep 18 20:57:55 primary-ws kernel: RDX: 0001 RSI:
958d255c RDI: 
Sep 18 20:57:55 primary-ws kernel: RBP: 8e19a83f5600 R08:
 R09: a1a853ccbd10
Sep 18 20:57:55 primary-ws kernel: R10: 0003 R11:
8e19ee2fffe8 R12: 8e19a83fc800
Sep 18 

Re: [PATCH] drm/amd/display: remove redundant CalculateRemoteSurfaceFlipDelay's

2022-09-19 Thread Maíra Canal
Hi Tom

On 9/19/22 14:27, Tom Rix wrote:
> There are several copies of CalculateRemoteSurfaceFlipDelay.
> Reduce to one instance.
> 
> Signed-off-by: Tom Rix 

Reviewed-by: Maíra Canal 

Just a minor comment below.

> ---
>  .../dc/dml/dcn20/display_mode_vba_20.c|  4 +-
>  .../dc/dml/dcn20/display_mode_vba_20v2.c  | 40 +--
>  .../dc/dml/dcn21/display_mode_vba_21.c| 40 +--
>  3 files changed, 4 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> index 4ca080950924..8e5d58336bc5 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> @@ -158,7 +158,7 @@ double CalculateTWait(
>   double DRAMClockChangeLatency,
>   double UrgentLatency,
>   double SREnterPlusExitTime);
> -static double CalculateRemoteSurfaceFlipDelay(
> +double CalculateRemoteSurfaceFlipDelay(
>   struct display_mode_lib *mode_lib,
>   double VRatio,
>   double SwathWidth,
> @@ -2909,7 +2909,7 @@ double CalculateTWait(
>   }
>  }
>  
> -static double CalculateRemoteSurfaceFlipDelay(
> +double CalculateRemoteSurfaceFlipDelay(

I guess it would be more clear if this function was placed on the
display_mode_vba20.h and named dml20_CalculateRemoteSurfaceFlipDelay.
Then, it would be clearer that this function is shared over the DCN20s.

Best Regards,
- Maíra Canal

>   struct display_mode_lib *mode_lib,
>   double VRatio,
>   double SwathWidth,
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> index 2b4dcae4e432..e9ebc81adc71 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> @@ -182,7 +182,7 @@ double CalculateTWait(
>   double DRAMClockChangeLatency,
>   double UrgentLatency,
>   double SREnterPlusExitTime);
> -static double CalculateRemoteSurfaceFlipDelay(
> +double CalculateRemoteSurfaceFlipDelay(
>   struct display_mode_lib *mode_lib,
>   double VRatio,
>   double SwathWidth,
> @@ -2967,44 +2967,6 @@ static void dml20v2_DisplayPipeConfiguration(struct 
> display_mode_lib *mode_lib)
>   }
>  }
>  
> -static double CalculateRemoteSurfaceFlipDelay(
> - struct display_mode_lib *mode_lib,
> - double VRatio,
> - double SwathWidth,
> - double Bpp,
> - double LineTime,
> - double XFCTSlvVupdateOffset,
> - double XFCTSlvVupdateWidth,
> - double XFCTSlvVreadyOffset,
> - double XFCXBUFLatencyTolerance,
> - double XFCFillBWOverhead,
> - double XFCSlvChunkSize,
> - double XFCBusTransportTime,
> - double TCalc,
> - double TWait,
> - double *SrcActiveDrainRate,
> - double *TInitXFill,
> - double *TslvChk)
> -{
> - double TSlvSetup, AvgfillRate, result;
> -
> - *SrcActiveDrainRate = VRatio * SwathWidth * Bpp / LineTime;
> - TSlvSetup = XFCTSlvVupdateOffset + XFCTSlvVupdateWidth + 
> XFCTSlvVreadyOffset;
> - *TInitXFill = XFCXBUFLatencyTolerance / (1 + XFCFillBWOverhead / 100);
> - AvgfillRate = *SrcActiveDrainRate * (1 + XFCFillBWOverhead / 100);
> - *TslvChk = XFCSlvChunkSize / AvgfillRate;
> - dml_print(
> - "DML::CalculateRemoteSurfaceFlipDelay: 
> SrcActiveDrainRate: %f\n",
> - *SrcActiveDrainRate);
> - dml_print("DML::CalculateRemoteSurfaceFlipDelay: TSlvSetup: %f\n", 
> TSlvSetup);
> - dml_print("DML::CalculateRemoteSurfaceFlipDelay: TInitXFill: %f\n", 
> *TInitXFill);
> - dml_print("DML::CalculateRemoteSurfaceFlipDelay: AvgfillRate: %f\n", 
> AvgfillRate);
> - dml_print("DML::CalculateRemoteSurfaceFlipDelay: TslvChk: %f\n", 
> *TslvChk);
> - result = 2 * XFCBusTransportTime + TSlvSetup + TCalc + TWait + *TslvChk 
> + *TInitXFill; // TODO: This doesn't seem to match programming guide
> - dml_print("DML::CalculateRemoteSurfaceFlipDelay: 
> RemoteSurfaceFlipDelay: %f\n", result);
> - return result;
> -}
> -
>  static void CalculateActiveRowBandwidth(
>   bool GPUVMEnable,
>   enum source_format_class SourcePixelFormat,
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
> index a3ef3638d979..d94aaf899f9b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
> @@ -210,7 +210,7 @@ double 

Re: [PATCH] drm/amd/display: remove redundant CalculateTWait's

2022-09-19 Thread Maíra Canal
Hi Tom,

On 9/18/22 23:37, Tom Rix wrote:
> There are several copies of CalculateTwait.
> Reduce to one instance and change local variable name to match common usage.
> 
> Signed-off-by: Tom Rix 

Reviewed-by: Maíra Canal 

Although, it would be nice to put this function on the
display_mode_vba.h file, as all DCNs use this function.

Best Regards,
- Maíra Canal

> ---
>  .../dc/dml/dcn20/display_mode_vba_20.c| 16 +++---
>  .../dc/dml/dcn20/display_mode_vba_20v2.c  | 21 ++-
>  .../dc/dml/dcn21/display_mode_vba_21.c| 19 +
>  .../dc/dml/dcn30/display_mode_vba_30.c| 18 +---
>  .../dc/dml/dcn31/display_mode_vba_31.c| 13 +---
>  .../dc/dml/dcn314/display_mode_vba_314.c  | 13 +---
>  6 files changed, 14 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> index 6e9d7e2b5243..4ca080950924 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> @@ -153,10 +153,10 @@ static unsigned int CalculateVMAndRowBytes(
>   bool *PTEBufferSizeNotExceeded,
>   unsigned int *dpte_row_height,
>   unsigned int *meta_row_height);
> -static double CalculateTWait(
> +double CalculateTWait(
>   unsigned int PrefetchMode,
>   double DRAMClockChangeLatency,
> - double UrgentLatencyPixelDataOnly,
> + double UrgentLatency,
>   double SREnterPlusExitTime);
>  static double CalculateRemoteSurfaceFlipDelay(
>   struct display_mode_lib *mode_lib,
> @@ -2892,20 +2892,20 @@ static void dml20_DisplayPipeConfiguration(struct 
> display_mode_lib *mode_lib)
>   }
>  }
>  
> -static double CalculateTWait(
> +double CalculateTWait(
>   unsigned int PrefetchMode,
>   double DRAMClockChangeLatency,
> - double UrgentLatencyPixelDataOnly,
> + double UrgentLatency,
>   double SREnterPlusExitTime)
>  {
>   if (PrefetchMode == 0) {
>   return dml_max(
> - DRAMClockChangeLatency + 
> UrgentLatencyPixelDataOnly,
> - dml_max(SREnterPlusExitTime, 
> UrgentLatencyPixelDataOnly));
> + DRAMClockChangeLatency + UrgentLatency,
> + dml_max(SREnterPlusExitTime, UrgentLatency));
>   } else if (PrefetchMode == 1) {
> - return dml_max(SREnterPlusExitTime, UrgentLatencyPixelDataOnly);
> + return dml_max(SREnterPlusExitTime, UrgentLatency);
>   } else {
> - return UrgentLatencyPixelDataOnly;
> + return UrgentLatency;
>   }
>  }
>  
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> index b02dda8ce70f..2b4dcae4e432 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
> @@ -177,10 +177,10 @@ static unsigned int CalculateVMAndRowBytes(
>   bool *PTEBufferSizeNotExceeded,
>   unsigned int *dpte_row_height,
>   unsigned int *meta_row_height);
> -static double CalculateTWait(
> +double CalculateTWait(
>   unsigned int PrefetchMode,
>   double DRAMClockChangeLatency,
> - double UrgentLatencyPixelDataOnly,
> + double UrgentLatency,
>   double SREnterPlusExitTime);
>  static double CalculateRemoteSurfaceFlipDelay(
>   struct display_mode_lib *mode_lib,
> @@ -2967,23 +2967,6 @@ static void dml20v2_DisplayPipeConfiguration(struct 
> display_mode_lib *mode_lib)
>   }
>  }
>  
> -static double CalculateTWait(
> - unsigned int PrefetchMode,
> - double DRAMClockChangeLatency,
> - double UrgentLatencyPixelDataOnly,
> - double SREnterPlusExitTime)
> -{
> - if (PrefetchMode == 0) {
> - return dml_max(
> - DRAMClockChangeLatency + 
> UrgentLatencyPixelDataOnly,
> - dml_max(SREnterPlusExitTime, 
> UrgentLatencyPixelDataOnly));
> - } else if (PrefetchMode == 1) {
> - return dml_max(SREnterPlusExitTime, UrgentLatencyPixelDataOnly);
> - } else {
> - return UrgentLatencyPixelDataOnly;
> - }
> -}
> -
>  static double CalculateRemoteSurfaceFlipDelay(
>   struct display_mode_lib *mode_lib,
>   double VRatio,
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
> index 6be14f55c78d..a3ef3638d979 100644
> --- 

Re: [PATCH] drm/amd/display: refactor CalculateWriteBackDelay to use vba_vars_st ptr

2022-09-19 Thread Maíra Canal
Hi Tom

Nice to see this patch coming to the DML! Some small nits inline.

On 9/17/22 15:37, Tom Rix wrote:
> Mimimize the function signature by passing a pointer and an index instead
> of passing several elements of the pointer.
> 
> The dml2x,dml3x families uses the same algorithm.  Remove the duplicates.
> Use dml20_ and dml30_ prefix to distinguish the two variants.
> 
> Signed-off-by: Tom Rix 
> ---
>  .../dc/dml/dcn20/display_mode_vba_20.c|  78 +++-
>  .../dc/dml/dcn20/display_mode_vba_20v2.c  | 115 ++
>  .../dc/dml/dcn21/display_mode_vba_21.c| 114 +
>  .../dc/dml/dcn30/display_mode_vba_30.c|  74 +++
>  .../dc/dml/dcn31/display_mode_vba_31.c|  76 +---
>  .../dc/dml/dcn314/display_mode_vba_314.c  |  76 +---
>  .../dc/dml/dcn32/display_mode_vba_32.c|  42 +--
>  .../dc/dml/dcn32/display_mode_vba_util_32.c   |  30 -
>  .../dc/dml/dcn32/display_mode_vba_util_32.h   |  10 +-
>  9 files changed, 63 insertions(+), 552 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> index d3b5b6fedf04..6e9d7e2b5243 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
> @@ -217,16 +217,8 @@ static void CalculateFlipSchedule(
>   double *DestinationLinesToRequestRowInImmediateFlip,
>   double *final_flip_bw,
>   bool *ImmediateFlipSupportedForPipe);
> -static double CalculateWriteBackDelay(
> - enum source_format_class WritebackPixelFormat,
> - double WritebackHRatio,
> - double WritebackVRatio,
> - unsigned int WritebackLumaHTaps,
> - unsigned int WritebackLumaVTaps,
> - unsigned int WritebackChromaHTaps,
> - unsigned int WritebackChromaVTaps,
> - unsigned int WritebackDestinationWidth);
>  
> +double dlm20_CalculateWriteBackDelay(struct vba_vars_st *vba, unsigned int 
> i);

Small typo here: s/dlm/dml

>  static void dml20_DisplayPipeConfiguration(struct display_mode_lib 
> *mode_lib);
>  static void 
> dml20_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
>   struct display_mode_lib *mode_lib);
> @@ -1085,6 +1077,7 @@ static unsigned int CalculateVMAndRowBytes(
>  static void 
> dml20_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
>   struct display_mode_lib *mode_lib)
>  {
> + struct vba_vars_st *v = _lib->vba;
>   unsigned int j, k;
>  
>   mode_lib->vba.WritebackDISPCLK = 0.0;
> @@ -1980,36 +1973,15 @@ static void 
> dml20_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPer
>   if (mode_lib->vba.BlendingAndTiming[k] == k) {
>   if (mode_lib->vba.WritebackEnable[k] == true) {
>   
> mode_lib->vba.WritebackDelay[mode_lib->vba.VoltageLevel][k] =
> - mode_lib->vba.WritebackLatency
> - + 
> CalculateWriteBackDelay(
> - 
> mode_lib->vba.WritebackPixelFormat[k],
> - 
> mode_lib->vba.WritebackHRatio[k],
> - 
> mode_lib->vba.WritebackVRatio[k],
> - 
> mode_lib->vba.WritebackLumaHTaps[k],
> - 
> mode_lib->vba.WritebackLumaVTaps[k],
> - 
> mode_lib->vba.WritebackChromaHTaps[k],
> - 
> mode_lib->vba.WritebackChromaVTaps[k],
> - 
> mode_lib->vba.WritebackDestinationWidth[k])
> - 
> / mode_lib->vba.DISPCLK;
> + mode_lib->vba.WritebackLatency + 
> dlm20_CalculateWriteBackDelay(v, k) / mode_lib->vba.DISPCLK;
>   } else
>   
> mode_lib->vba.WritebackDelay[mode_lib->vba.VoltageLevel][k] = 0;
>   for (j = 0; j < mode_lib->vba.NumberOfActivePlanes; 
> ++j) {
>   if (mode_lib->vba.BlendingAndTiming[j] == k
>   && 
> mode_lib->vba.WritebackEnable[j] == true) {
>   
> mode_lib->vba.WritebackDelay[mode_lib->vba.VoltageLevel][k] =
> -  

Re: [PATCH 1/4] drm/amdgpu: Introduce gfx software ring(v4)

2022-09-19 Thread Luben Tuikov
Hi,

You can still use "writep" and "readp" in the function name. Make code 
sustainable for the next programmer in the future.

Regards,
Luben

On 2022-09-15 01:30, Zhu, Jiadong wrote:
> [AMD Official Use Only - General]
> 
>> So, between the function names of "amdgpu_ring_get_rptr_from_mux()" and 
>> "amdgpu_ring_get_wptr_from_mux()",
>> they are 96.6(6)% _the_same_ name to a human. How about 
>> "amdgpu_ring_get_readp_from_mux()" and "amdgpu_ring_get_writep_from_mux()"?
> 
> I agree with the similarity between rptr and wptr. But the function named 
> amdgpu_ring_get_rptr_from_mux  is aligned with amdgpu_ring_get_rptr for the 
> same style, easier to understand.
> I would rather use it currently until we replace rptr with writep in all the 
> other places, shall we?
> 
> Thanks,
> Jiadong
> 
> -Original Message-
> From: Tuikov, Luben 
> Sent: Thursday, September 15, 2022 12:49 AM
> To: Zhu, Jiadong ; amd-gfx@lists.freedesktop.org
> Cc: Huang, Ray ; Grodzovsky, Andrey 
> ; Koenig, Christian 
> Subject: Re: [PATCH 1/4] drm/amdgpu: Introduce gfx software ring(v4)
> 
> It's customary to add a Cc: tag in the commit message on subsequent revisions 
> of a patch, once a person has reviewed and commented on it--Christian, 
> Andrey, me, so that subsequent patches' emails go out to those people 
> automatically via a CC.
> 
> Inlined:
> 
> On 2022-09-13 05:05, jiadong@amd.com wrote:
>> From: "Jiadong.Zhu" 
>>
>> The software ring is created to support priority context while there
>> is only one hardware queue for gfx.
>>
>> Every software rings has its fence driver and could
> 
> "ring", singular.
> 
>> be used as an ordinary ring for the gpu_scheduler.
> 
> "GPU scheduler".
> 
>> Multiple software rings are binded to a real ring
> 
> The past tense of "bind" is "bound", not "binded".
> 
>> with the ring muxer. The packages committed on the software ring are
>> copied to the real ring.
> 
> Use 79 column wrap setting in your editor, not 50 or 60.
> Wrap at 79.
> 
>>
>> v2: use array to store software ring entry.
>> v3: remove unnecessary prints.
>> v4: remove amdgpu_ring_sw_init/fini functions, using gtt for sw ring
>> buffer for later dma copy optimization.
>>
>> Signed-off-by: Jiadong.Zhu 
> 
> Add Cc: tags before the Signed-off-by line.
> 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/Makefile  |   3 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h  |   3 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |   3 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c | 182
>> +++  drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.h |
>> 67 +++  drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.c  |  60 ++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.h  |  43 +
>>  7 files changed, 360 insertions(+), 1 deletion(-)  create mode 100644
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c
>>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.h
>>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.c
>>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sw_ring.h
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>> index 3e0e2eb7e235..85224bc81ce5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>> @@ -58,7 +58,8 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>>   amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
>>   amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
>>   amdgpu_fw_attestation.o amdgpu_securedisplay.o \
>> - amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o
>> + amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \
>> + amdgpu_sw_ring.o amdgpu_ring_mux.o
>>
>>  amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> index 53526ffb2ce1..0de8e3cd0f1c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> @@ -33,6 +33,7 @@
>>  #include "amdgpu_imu.h"
>>  #include "soc15.h"
>>  #include "amdgpu_ras.h"
>> +#include "amdgpu_ring_mux.h"
>>
>>  /* GFX current status */
>>  #define AMDGPU_GFX_NORMAL_MODE   0xL
>> @@ -346,6 +347,8 @@ struct amdgpu_gfx {
>>   struct amdgpu_gfx_ras   *ras;
>>
>>   boolis_poweron;
>> +
>> + struct amdgpu_ring_mux  muxer;
> 
> It doesn't align, possibly because TAB chars were used between "bool" and 
> "is_poweron", and because TAB chars were used between "struct 
> amdgpu_ring_mux" and "muxer".
> 
>>  };
>>
>>  #define amdgpu_gfx_get_gpu_clock_counter(adev)
>> (adev)->gfx.funcs->get_gpu_clock_counter((adev))
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> index 7d89a52091c0..fe33a683bfba 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> +++ 

[PATCH] drm/amd/display: remove redundant CalculateRemoteSurfaceFlipDelay's

2022-09-19 Thread Tom Rix
There are several copies of CalculateRemoteSurfaceFlipDelay.
Reduce to one instance.

Signed-off-by: Tom Rix 
---
 .../dc/dml/dcn20/display_mode_vba_20.c|  4 +-
 .../dc/dml/dcn20/display_mode_vba_20v2.c  | 40 +--
 .../dc/dml/dcn21/display_mode_vba_21.c| 40 +--
 3 files changed, 4 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
index 4ca080950924..8e5d58336bc5 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
@@ -158,7 +158,7 @@ double CalculateTWait(
double DRAMClockChangeLatency,
double UrgentLatency,
double SREnterPlusExitTime);
-static double CalculateRemoteSurfaceFlipDelay(
+double CalculateRemoteSurfaceFlipDelay(
struct display_mode_lib *mode_lib,
double VRatio,
double SwathWidth,
@@ -2909,7 +2909,7 @@ double CalculateTWait(
}
 }
 
-static double CalculateRemoteSurfaceFlipDelay(
+double CalculateRemoteSurfaceFlipDelay(
struct display_mode_lib *mode_lib,
double VRatio,
double SwathWidth,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
index 2b4dcae4e432..e9ebc81adc71 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
@@ -182,7 +182,7 @@ double CalculateTWait(
double DRAMClockChangeLatency,
double UrgentLatency,
double SREnterPlusExitTime);
-static double CalculateRemoteSurfaceFlipDelay(
+double CalculateRemoteSurfaceFlipDelay(
struct display_mode_lib *mode_lib,
double VRatio,
double SwathWidth,
@@ -2967,44 +2967,6 @@ static void dml20v2_DisplayPipeConfiguration(struct 
display_mode_lib *mode_lib)
}
 }
 
-static double CalculateRemoteSurfaceFlipDelay(
-   struct display_mode_lib *mode_lib,
-   double VRatio,
-   double SwathWidth,
-   double Bpp,
-   double LineTime,
-   double XFCTSlvVupdateOffset,
-   double XFCTSlvVupdateWidth,
-   double XFCTSlvVreadyOffset,
-   double XFCXBUFLatencyTolerance,
-   double XFCFillBWOverhead,
-   double XFCSlvChunkSize,
-   double XFCBusTransportTime,
-   double TCalc,
-   double TWait,
-   double *SrcActiveDrainRate,
-   double *TInitXFill,
-   double *TslvChk)
-{
-   double TSlvSetup, AvgfillRate, result;
-
-   *SrcActiveDrainRate = VRatio * SwathWidth * Bpp / LineTime;
-   TSlvSetup = XFCTSlvVupdateOffset + XFCTSlvVupdateWidth + 
XFCTSlvVreadyOffset;
-   *TInitXFill = XFCXBUFLatencyTolerance / (1 + XFCFillBWOverhead / 100);
-   AvgfillRate = *SrcActiveDrainRate * (1 + XFCFillBWOverhead / 100);
-   *TslvChk = XFCSlvChunkSize / AvgfillRate;
-   dml_print(
-   "DML::CalculateRemoteSurfaceFlipDelay: 
SrcActiveDrainRate: %f\n",
-   *SrcActiveDrainRate);
-   dml_print("DML::CalculateRemoteSurfaceFlipDelay: TSlvSetup: %f\n", 
TSlvSetup);
-   dml_print("DML::CalculateRemoteSurfaceFlipDelay: TInitXFill: %f\n", 
*TInitXFill);
-   dml_print("DML::CalculateRemoteSurfaceFlipDelay: AvgfillRate: %f\n", 
AvgfillRate);
-   dml_print("DML::CalculateRemoteSurfaceFlipDelay: TslvChk: %f\n", 
*TslvChk);
-   result = 2 * XFCBusTransportTime + TSlvSetup + TCalc + TWait + *TslvChk 
+ *TInitXFill; // TODO: This doesn't seem to match programming guide
-   dml_print("DML::CalculateRemoteSurfaceFlipDelay: 
RemoteSurfaceFlipDelay: %f\n", result);
-   return result;
-}
-
 static void CalculateActiveRowBandwidth(
bool GPUVMEnable,
enum source_format_class SourcePixelFormat,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
index a3ef3638d979..d94aaf899f9b 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
@@ -210,7 +210,7 @@ double CalculateTWait(
double DRAMClockChangeLatency,
double UrgentLatency,
double SREnterPlusExitTime);
-static double CalculateRemoteSurfaceFlipDelay(
+double CalculateRemoteSurfaceFlipDelay(
struct display_mode_lib *mode_lib,
double VRatio,
double SwathWidth,
@@ -2980,44 +2980,6 @@ static void DisplayPipeConfiguration(struct 
display_mode_lib *mode_lib)
   

[PATCH v2 2/7] drm/amdgpu: Use vm status_lock to protect relocated list

2022-09-19 Thread Philip Yang
Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 596f1ea8babc..4a1cb20deb2d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -240,10 +240,13 @@ static void amdgpu_vm_bo_invalidated(struct 
amdgpu_vm_bo_base *vm_bo)
  */
 static void amdgpu_vm_bo_relocated(struct amdgpu_vm_bo_base *vm_bo)
 {
-   if (vm_bo->bo->parent)
+   if (vm_bo->bo->parent) {
+   spin_lock(_bo->vm->status_lock);
list_move(_bo->vm_status, _bo->vm->relocated);
-   else
+   spin_unlock(_bo->vm->status_lock);
+   } else {
amdgpu_vm_bo_idle(vm_bo);
+   }
 }
 
 /**
@@ -680,9 +683,14 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
struct amdgpu_vm_update_params params;
struct amdgpu_vm_bo_base *entry;
bool flush_tlb_needed = false;
+   LIST_HEAD(relocated);
int r, idx;
 
-   if (list_empty(>relocated))
+   spin_lock(>status_lock);
+   list_splice_init(>relocated, );
+   spin_unlock(>status_lock);
+
+   if (list_empty())
return 0;
 
if (!drm_dev_enter(adev_to_drm(adev), ))
@@ -697,7 +705,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
if (r)
goto error;
 
-   list_for_each_entry(entry, >relocated, vm_status) {
+   list_for_each_entry(entry, , vm_status) {
/* vm_flush_needed after updating moved PDEs */
flush_tlb_needed |= entry->moved;
 
@@ -713,9 +721,8 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
if (flush_tlb_needed)
atomic64_inc(>tlb_seq);
 
-   while (!list_empty(>relocated)) {
-   entry = list_first_entry(>relocated,
-struct amdgpu_vm_bo_base,
+   while (!list_empty()) {
+   entry = list_first_entry(, struct amdgpu_vm_bo_base,
 vm_status);
amdgpu_vm_bo_idle(entry);
}
@@ -912,6 +919,7 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t 
*vram_mem,
 {
struct amdgpu_bo_va *bo_va, *tmp;
 
+   spin_lock(>status_lock);
list_for_each_entry_safe(bo_va, tmp, >idle, base.vm_status) {
if (!bo_va->base.bo)
continue;
@@ -936,7 +944,6 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t 
*vram_mem,
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
gtt_mem, cpu_mem);
}
-   spin_lock(>status_lock);
list_for_each_entry_safe(bo_va, tmp, >invalidated, base.vm_status) {
if (!bo_va->base.bo)
continue;
-- 
2.35.1



[PATCH v2 7/7] drm/amdgpu: Fix amdgpu_vm_pt_free warning

2022-09-19 Thread Philip Yang
Free page table BO from vm resv unlocked context generate below
warnings.

Add a pt_free_work in vm to free page table BO from vm->pt_freed list.
pass vm resv unlock status from page table update caller, and add vm_bo
entry to vm->pt_freed list and schedule the pt_free_work if calling with
vm resv unlocked.

WARNING: CPU: 12 PID: 3238 at
drivers/gpu/drm/ttm/ttm_bo.c:106 ttm_bo_set_bulk_move+0xa1/0xc0
Call Trace:
 amdgpu_vm_pt_free+0x42/0xd0 [amdgpu]
 amdgpu_vm_pt_free_dfs+0xb3/0xf0 [amdgpu]
 amdgpu_vm_ptes_update+0x52d/0x850 [amdgpu]
 amdgpu_vm_update_range+0x2a6/0x640 [amdgpu]
 svm_range_unmap_from_gpus+0x110/0x300 [amdgpu]
 svm_range_cpu_invalidate_pagetables+0x535/0x600 [amdgpu]
 __mmu_notifier_invalidate_range_start+0x1cd/0x230
 unmap_vmas+0x9d/0x140
 unmap_region+0xa8/0x110

Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|  5 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 41 +--
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index b2e96682b9bb..83b0c5d86e48 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2055,6 +2055,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
spin_lock_init(>status_lock);
INIT_LIST_HEAD(>freed);
INIT_LIST_HEAD(>done);
+   INIT_LIST_HEAD(>pt_freed);
+   INIT_WORK(>pt_free_work, amdgpu_vm_pt_free_work);
 
/* create scheduler entities for page table updates */
r = drm_sched_entity_init(>immediate, DRM_SCHED_PRIORITY_NORMAL,
@@ -2256,6 +2258,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 
amdgpu_amdkfd_gpuvm_destroy_cb(adev, vm);
 
+   flush_work(>pt_free_work);
+
root = amdgpu_bo_ref(vm->root.bo);
amdgpu_bo_reserve(root, true);
amdgpu_vm_set_pasid(adev, vm, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index e6dd112d865c..83acb7bd80fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -278,6 +278,10 @@ struct amdgpu_vm {
/* BOs which are invalidated, has been updated in the PTs */
struct list_headdone;
 
+   /* PT BOs scheduled to free and fill with zero if vm_resv is not hold */
+   struct list_headpt_freed;
+   struct work_struct  pt_free_work;
+
/* contains the page directory */
struct amdgpu_vm_bo_base root;
struct dma_fence*last_update;
@@ -473,6 +477,7 @@ int amdgpu_vm_pde_update(struct amdgpu_vm_update_params 
*params,
 int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
  uint64_t start, uint64_t end,
  uint64_t dst, uint64_t flags);
+void amdgpu_vm_pt_free_work(struct work_struct *work);
 
 #if defined(CONFIG_DEBUG_FS)
 void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 61a4b7182d44..358b91243e37 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -644,6 +644,27 @@ static void amdgpu_vm_pt_free(struct amdgpu_vm_bo_base 
*entry)
amdgpu_bo_unref(>bo);
 }
 
+void amdgpu_vm_pt_free_work(struct work_struct *work)
+{
+   struct amdgpu_vm_bo_base *entry, *next;
+   struct amdgpu_vm *vm;
+   LIST_HEAD(pt_freed);
+
+   vm = container_of(work, struct amdgpu_vm, pt_free_work);
+
+   spin_lock(>status_lock);
+   list_splice_init(>pt_freed, _freed);
+   spin_unlock(>status_lock);
+
+   /* flush_work in amdgpu_vm_fini ensure vm->root.bo is valid. */
+   amdgpu_bo_reserve(vm->root.bo, true);
+
+   list_for_each_entry_safe(entry, next, _freed, vm_status)
+   amdgpu_vm_pt_free(entry);
+
+   amdgpu_bo_unreserve(vm->root.bo);
+}
+
 /**
  * amdgpu_vm_pt_free_dfs - free PD/PT levels
  *
@@ -655,11 +676,24 @@ static void amdgpu_vm_pt_free(struct amdgpu_vm_bo_base 
*entry)
  */
 static void amdgpu_vm_pt_free_dfs(struct amdgpu_device *adev,
  struct amdgpu_vm *vm,
- struct amdgpu_vm_pt_cursor *start)
+ struct amdgpu_vm_pt_cursor *start,
+ bool unlocked)
 {
struct amdgpu_vm_pt_cursor cursor;
struct amdgpu_vm_bo_base *entry;
 
+   if (unlocked) {
+   spin_lock(>status_lock);
+   for_each_amdgpu_vm_pt_dfs_safe(adev, vm, start, cursor, entry)
+   list_move(>vm_status, >pt_freed);
+
+   if (start)
+   list_move(>entry->vm_status, >pt_freed);
+   spin_unlock(>status_lock);
+   

[PATCH v2 4/7] drm/amdgpu: Use vm status_lock to protect vm moved list

2022-09-19 Thread Philip Yang
Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c3412709e626..168875115538 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -198,7 +198,9 @@ static void amdgpu_vm_bo_evicted(struct amdgpu_vm_bo_base 
*vm_bo)
  */
 static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base *vm_bo)
 {
+   spin_lock(_bo->vm->status_lock);
list_move(_bo->vm_status, _bo->vm->moved);
+   spin_unlock(_bo->vm->status_lock);
 }
 
 /**
@@ -1287,19 +1289,24 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
   struct amdgpu_vm *vm)
 {
-   struct amdgpu_bo_va *bo_va, *tmp;
+   struct amdgpu_bo_va *bo_va;
struct dma_resv *resv;
bool clear;
int r;
 
-   list_for_each_entry_safe(bo_va, tmp, >moved, base.vm_status) {
+   spin_lock(>status_lock);
+   while (!list_empty(>moved)) {
+   bo_va = list_first_entry(>moved, struct amdgpu_bo_va,
+base.vm_status);
+   spin_unlock(>status_lock);
+
/* Per VM BOs never need to bo cleared in the page tables */
r = amdgpu_vm_bo_update(adev, bo_va, false);
if (r)
return r;
+   spin_lock(>status_lock);
}
 
-   spin_lock(>status_lock);
while (!list_empty(>invalidated)) {
bo_va = list_first_entry(>invalidated, struct amdgpu_bo_va,
 base.vm_status);
@@ -1396,7 +1403,7 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device 
*adev,
 
if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
!bo_va->base.moved) {
-   list_move(_va->base.vm_status, >moved);
+   amdgpu_vm_bo_moved(_va->base);
}
trace_amdgpu_vm_bo_map(bo_va, mapping);
 }
-- 
2.35.1



[PATCH v2 5/7] drm/amdgpu: Use vm status_lock to protect vm evicted list

2022-09-19 Thread Philip Yang
Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 27 +-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 168875115538..b2e96682b9bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -183,10 +183,12 @@ static void amdgpu_vm_bo_evicted(struct amdgpu_vm_bo_base 
*vm_bo)
struct amdgpu_bo *bo = vm_bo->bo;
 
vm_bo->moved = true;
+   spin_lock(_bo->vm->status_lock);
if (bo->tbo.type == ttm_bo_type_kernel)
list_move(_bo->vm_status, >evicted);
else
list_move_tail(_bo->vm_status, >evicted);
+   spin_unlock(_bo->vm->status_lock);
 }
 /**
  * amdgpu_vm_bo_moved - vm_bo is moved
@@ -370,12 +372,20 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
  int (*validate)(void *p, struct amdgpu_bo *bo),
  void *param)
 {
-   struct amdgpu_vm_bo_base *bo_base, *tmp;
+   struct amdgpu_vm_bo_base *bo_base;
+   struct amdgpu_bo *shadow;
+   struct amdgpu_bo *bo;
int r;
 
-   list_for_each_entry_safe(bo_base, tmp, >evicted, vm_status) {
-   struct amdgpu_bo *bo = bo_base->bo;
-   struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
+   spin_lock(>status_lock);
+   while (!list_empty(>evicted)) {
+   bo_base = list_first_entry(>evicted,
+  struct amdgpu_vm_bo_base,
+  vm_status);
+   spin_unlock(>status_lock);
+
+   bo = bo_base->bo;
+   shadow = amdgpu_bo_shadowed(bo);
 
r = validate(param, bo);
if (r)
@@ -392,7 +402,9 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
vm->update_funcs->map_table(to_amdgpu_bo_vm(bo));
amdgpu_vm_bo_relocated(bo_base);
}
+   spin_lock(>status_lock);
}
+   spin_unlock(>status_lock);
 
amdgpu_vm_eviction_lock(vm);
vm->evicting = false;
@@ -413,13 +425,18 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
  */
 bool amdgpu_vm_ready(struct amdgpu_vm *vm)
 {
+   bool empty;
bool ret;
 
amdgpu_vm_eviction_lock(vm);
ret = !vm->evicting;
amdgpu_vm_eviction_unlock(vm);
 
-   return ret && list_empty(>evicted);
+   spin_lock(>status_lock);
+   empty = list_empty(>evicted);
+   spin_unlock(>status_lock);
+
+   return ret && empty;
 }
 
 /**
-- 
2.35.1



[PATCH v2 0/7] Fix amdgpu_vm_pt_free warning

2022-09-19 Thread Philip Yang
When SVM range is unmapped from CPU, the mmu notifier callback update 
GPU page table to unmap the SVM range from GPU, this is the unlocked 
page table update context as we cannot take vm reservation lock. If 
unmapping from GPU free pt bo, this cause warning as vm reservation lock 
is not hold.

To fix the warning/race issue, we schedule pt_free_work to free pt bo. 
This has another race to remove vm_bo entry from vm status lists. Some 
of the vm status lists are protected by vm invalidate lock and the rest 
of lists are protected by vm reservation lock.

The purpose of this patch series is to use one vm status_lock to protect 
all vm status lists, and use it in pt_free_work to fix the race and 
remove the warning.

v2: Correct comments style, move status_lock definition before all the 
list_head members

Philip Yang (7):
  drm/amdgpu: Rename vm invalidate lock to status_lock
  drm/amdgpu: Use vm status_lock to protect relocated list
  drm/amdgpu: Use vm status_lock to protect vm idle list
  drm/amdgpu: Use vm status_lock to protect vm moved list
  drm/amdgpu: Use vm status_lock to protect vm evicted list
  drm/amdgpu: Use vm status_lock to protect pt free
  drm/amdgpu: Fix amdgpu_vm_pt_free warning

 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 97 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|  9 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 44 +-
 3 files changed, 116 insertions(+), 34 deletions(-)

-- 
2.35.1



[PATCH v2 3/7] drm/amdgpu: Use vm status_lock to protect vm idle list

2022-09-19 Thread Philip Yang
Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 4a1cb20deb2d..c3412709e626 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -211,7 +211,9 @@ static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base 
*vm_bo)
  */
 static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base *vm_bo)
 {
+   spin_lock(_bo->vm->status_lock);
list_move(_bo->vm_status, _bo->vm->idle);
+   spin_unlock(_bo->vm->status_lock);
vm_bo->moved = false;
 }
 
@@ -2554,6 +2556,7 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, 
struct seq_file *m)
unsigned int total_done_objs = 0;
unsigned int id = 0;
 
+   spin_lock(>status_lock);
seq_puts(m, "\tIdle BOs:\n");
list_for_each_entry_safe(bo_va, tmp, >idle, base.vm_status) {
if (!bo_va->base.bo)
@@ -2591,7 +2594,6 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, 
struct seq_file *m)
id = 0;
 
seq_puts(m, "\tInvalidated BOs:\n");
-   spin_lock(>status_lock);
list_for_each_entry_safe(bo_va, tmp, >invalidated, base.vm_status) {
if (!bo_va->base.bo)
continue;
-- 
2.35.1



[PATCH v2 6/7] drm/amdgpu: Use vm status_lock to protect pt free

2022-09-19 Thread Philip Yang
Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 88de9f0d4728..61a4b7182d44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -637,7 +637,10 @@ static void amdgpu_vm_pt_free(struct amdgpu_vm_bo_base 
*entry)
}
ttm_bo_set_bulk_move(>bo->tbo, NULL);
entry->bo->vm_bo = NULL;
+
+   spin_lock(>vm->status_lock);
list_del(>vm_status);
+   spin_unlock(>vm->status_lock);
amdgpu_bo_unref(>bo);
 }
 
-- 
2.35.1



[PATCH v2 1/7] drm/amdgpu: Rename vm invalidate lock to status_lock

2022-09-19 Thread Philip Yang
The vm status_lock will be used to protect all vm status lists.

Signed-off-by: Philip Yang 
Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 30 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 +++-
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 690fd4f639f1..596f1ea8babc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -225,9 +225,9 @@ static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base 
*vm_bo)
  */
 static void amdgpu_vm_bo_invalidated(struct amdgpu_vm_bo_base *vm_bo)
 {
-   spin_lock(_bo->vm->invalidated_lock);
+   spin_lock(_bo->vm->status_lock);
list_move(_bo->vm_status, _bo->vm->invalidated);
-   spin_unlock(_bo->vm->invalidated_lock);
+   spin_unlock(_bo->vm->status_lock);
 }
 
 /**
@@ -256,9 +256,9 @@ static void amdgpu_vm_bo_relocated(struct amdgpu_vm_bo_base 
*vm_bo)
  */
 static void amdgpu_vm_bo_done(struct amdgpu_vm_bo_base *vm_bo)
 {
-   spin_lock(_bo->vm->invalidated_lock);
+   spin_lock(_bo->vm->status_lock);
list_move(_bo->vm_status, _bo->vm->done);
-   spin_unlock(_bo->vm->invalidated_lock);
+   spin_unlock(_bo->vm->status_lock);
 }
 
 /**
@@ -936,7 +936,7 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t 
*vram_mem,
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
gtt_mem, cpu_mem);
}
-   spin_lock(>invalidated_lock);
+   spin_lock(>status_lock);
list_for_each_entry_safe(bo_va, tmp, >invalidated, base.vm_status) {
if (!bo_va->base.bo)
continue;
@@ -949,7 +949,7 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t 
*vram_mem,
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
gtt_mem, cpu_mem);
}
-   spin_unlock(>invalidated_lock);
+   spin_unlock(>status_lock);
 }
 /**
  * amdgpu_vm_bo_update - update all BO mappings in the vm page table
@@ -1290,12 +1290,12 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
return r;
}
 
-   spin_lock(>invalidated_lock);
+   spin_lock(>status_lock);
while (!list_empty(>invalidated)) {
bo_va = list_first_entry(>invalidated, struct amdgpu_bo_va,
 base.vm_status);
resv = bo_va->base.bo->tbo.base.resv;
-   spin_unlock(>invalidated_lock);
+   spin_unlock(>status_lock);
 
/* Try to reserve the BO to avoid clearing its ptes */
if (!amdgpu_vm_debug && dma_resv_trylock(resv))
@@ -1310,9 +1310,9 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
 
if (!clear)
dma_resv_unlock(resv);
-   spin_lock(>invalidated_lock);
+   spin_lock(>status_lock);
}
-   spin_unlock(>invalidated_lock);
+   spin_unlock(>status_lock);
 
return 0;
 }
@@ -1763,9 +1763,9 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
}
}
 
-   spin_lock(>invalidated_lock);
+   spin_lock(>status_lock);
list_del(_va->base.vm_status);
-   spin_unlock(>invalidated_lock);
+   spin_unlock(>status_lock);
 
list_for_each_entry_safe(mapping, next, _va->valids, list) {
list_del(>list);
@@ -2019,7 +2019,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
INIT_LIST_HEAD(>moved);
INIT_LIST_HEAD(>idle);
INIT_LIST_HEAD(>invalidated);
-   spin_lock_init(>invalidated_lock);
+   spin_lock_init(>status_lock);
INIT_LIST_HEAD(>freed);
INIT_LIST_HEAD(>done);
 
@@ -2584,7 +2584,7 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, 
struct seq_file *m)
id = 0;
 
seq_puts(m, "\tInvalidated BOs:\n");
-   spin_lock(>invalidated_lock);
+   spin_lock(>status_lock);
list_for_each_entry_safe(bo_va, tmp, >invalidated, base.vm_status) {
if (!bo_va->base.bo)
continue;
@@ -2599,7 +2599,7 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, 
struct seq_file *m)
continue;
total_done += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
}
-   spin_unlock(>invalidated_lock);
+   spin_unlock(>status_lock);
total_done_objs = id;
 
seq_printf(m, "\tTotal idle size:%12lld\tobjs:\t%d\n", 
total_idle,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 9ecb7f663e19..e6dd112d865c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -254,6 +254,9 @@ struct amdgpu_vm {
boolevicting;
unsigned intsaved_flags;
 
+   

[PATCH] drm/amd/display: fix transfer function passed to build_coefficients()

2022-09-19 Thread Alex Deucher
The default argument should be enum TRANSFER_FUNCTION_SRGB rather than
the current boolean value which improperly maps to
TRANSFER_FUNCTION_BT709.

Commit 9b3d76527f6e ("drm/amd/display: Revert adding degamma coefficients")
looks to have improperly reverted
commit d02097095916 ("drm/amd/display: Add regamma/degamma coefficients and set 
sRGB when TF is BT709")
replacing the enum value with a boolean value.

Cc: Krunoslav Kovac 
Cc: Jaehyun Chung 
Cc: Zeng Heng 
Fixes: 9b3d76527f6e ("drm/amd/display: Revert adding degamma coefficients")
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c 
b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
index 04f7656906ca..447a0ec9cbe2 100644
--- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
+++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
@@ -1692,7 +1692,7 @@ static void apply_degamma_for_user_regamma(struct 
pwl_float_data_ex *rgb_regamma
struct pwl_float_data_ex *rgb = rgb_regamma;
const struct hw_x_point *coord_x = coordinates_x;
 
-   build_coefficients(, true);
+   build_coefficients(, TRANSFER_FUNCTION_SRGB);
 
i = 0;
while (i != hw_points_num + 1) {
-- 
2.37.3



Re: [PATCH] drm/amdgpu: use dirty framebuffer helper

2022-09-19 Thread Alex Deucher
On Sun, Sep 18, 2022 at 8:09 AM root  wrote:
>
> Hi, I recently experienced lock-ups that only responded to magic sysreq
> reboots when the amdgpu module was loading on my pc (Athlon II X4 640 CPU,
> with Radeon R7 250 - Cape Verde).
>
> .config has:
>
> CONFIG_DRM_AMDGPU=m
> CONFIG_DRM_AMDGPU_SI=y
> # CONFIG_DRM_AMDGPU_CIK is not set
> # CONFIG_DRM_AMDGPU_USERPTR is not set
>
> kernel command line has:
>
> amdgpu.audio=1 amdgpu.si_support=1 radeon.si_support=0 page_owner=on \
> amdgpu.gpu_recovery=1
>
> Bisecting lead to:
>
> commit 66f99628eb24409cb8feb5061f78283c8b65f820
> Author: Hamza Mahfooz 
> Date:   Tue Sep 6 15:01:49 2022 -0400
>
> drm/amdgpu: use dirty framebuffer helper
>
> Currently, we aren't handling DRM_IOCTL_MODE_DIRTYFB. So, use
> drm_atomic_helper_dirtyfb() as the dirty callback in the amdgpu_fb_funcs
> struct.
>
> Signed-off-by: Hamza Mahfooz 
> Acked-by: Alex Deucher 
> Signed-off-by: Alex Deucher 
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index c20922a5af9f..5b09c8f4fe95 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -38,6 +38,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -496,6 +497,7 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector 
> *amdgpu_connector,
>  static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
> .destroy = drm_gem_fb_destroy,
> .create_handle = drm_gem_fb_create_handle,
> +   .dirty = drm_atomic_helper_dirtyfb,
>  };
>
>  uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
>
> After doing a git bisect reset, git pull and reverting the patch above, I
> rebuilt the kernel and am successfully running with the amdgpu module loaded
> and using the Radeon R7 250 GPU.
>
> I am happy to supply any further configuration details.

Does the attached patch help?

Alex


>
> Arthur Marsh.
From 5e49c68c1ac1fbb98b1c84407485abfb7d309783 Mon Sep 17 00:00:00 2001
From: Alex Deucher 
Date: Mon, 19 Sep 2022 12:26:20 -0400
Subject: [PATCH] drm/amdgpu: don't register a dirty callback for non-atomic

Some asics still support non-atomic code paths.

Fixes: 66f99628eb2440 ("drm/amdgpu: use dirty framebuffer helper")
Reported-by: Arthur Marsh 
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 5b09c8f4fe95..23998f727c7f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -497,6 +498,11 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector *amdgpu_connector,
 static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
 	.destroy = drm_gem_fb_destroy,
 	.create_handle = drm_gem_fb_create_handle,
+};
+
+static const struct drm_framebuffer_funcs amdgpu_fb_funcs_atomic = {
+	.destroy = drm_gem_fb_destroy,
+	.create_handle = drm_gem_fb_create_handle,
 	.dirty = drm_atomic_helper_dirtyfb,
 };
 
@@ -1102,7 +1108,10 @@ static int amdgpu_display_gem_fb_verify_and_init(struct drm_device *dev,
 	if (ret)
 		goto err;
 
-	ret = drm_framebuffer_init(dev, >base, _fb_funcs);
+	if (drm_drv_uses_atomic_modeset(dev))
+		ret = drm_framebuffer_init(dev, >base, _fb_funcs_atomic);
+	else
+		ret = drm_framebuffer_init(dev, >base, _fb_funcs);
 	if (ret)
 		goto err;
 
-- 
2.37.3



RE: [PATCH] gpu: color: eliminate implicit conversion about enum type

2022-09-19 Thread Kovac, Krunoslav
[AMD Official Use Only - General]

> I think the proper fix is to set it to:
> build_coefficients(, TRANSFER_FUNCTION_SRGB);

I agree, default arg should be TRANSFER_FUNCTION_SRGB.
Even though it's a change in behaviour, previous behaviour was wrong.
Ideally it would be based on input TF, but afaik on both Linux and Win there's 
no current case where this is not sRGB.

Thanks,
Kruno

-Original Message-
From: Alex Deucher 
Sent: Monday, September 19, 2022 12:07 PM
To: Zeng Heng ; Chung, Jaehyun ; 
Kovac, Krunoslav 
Cc: Wentland, Harry ; Li, Sun peng (Leo) 
; Siqueira, Rodrigo ; Deucher, 
Alexander ; Koenig, Christian 
; Pan, Xinhui ; airl...@linux.ie; 
dan...@ffwll.ch; Kotarac, Pavle ; Liu, HaoPing (Alan) 
; Wang, Sherry ; 
weiyongj...@huawei.com; liwei...@huawei.com; dri-de...@lists.freedesktop.org; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] gpu: color: eliminate implicit conversion about enum type

Add the relevant AMD developers to comment.

On Mon, Sep 19, 2022 at 12:05 PM Alex Deucher  wrote:
>
> On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng  wrote:
> >
> > Fix below compile warning when open enum-conversion option check:
> >
> > drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:
> > In function ‘apply_degamma_for_user_regamma’:
> > drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1695:29:
> > error: implicit conversion from ‘enum ’ to ‘enum
> > dc_transfer_func_predefined’ [-Werror=enum-conversion]
> >  1695 |  build_coefficients(, true);
> >   | ^~~~
> >
> > As 'build_coefficients' definition, it needs enum
> > 'dc_transfer_func_predefined' type acts as the second argument,
> > instead of bool-type one.
> >
> > The numerical values of TRANSFER_FUNCTION_BT709 & true happen to be
> > the same, so there is no change in behavior.
>
> This looks like a regression from:
>
> commit 9b3d76527f6ea50270f7f7ac749493b41783e8bd
> Author: Jaehyun Chung 
> Date:   Mon Aug 30 16:46:42 2021 -0400
>
> drm/amd/display: Revert adding degamma coefficients
>
> [Why]
> Degamma coefficients are calculated in our degamma formula using
> the regamma coefficients. We do not need to add separate degamma
> coefficients.
>
> [How]
> Remove the change to add separate degamma coefficients.
>
> Reviewed-by: Krunoslav Kovac 
> Acked-by: Mikita Lipski 
> Signed-off-by: Jaehyun Chung 
> Tested-by: Daniel Wheeler 
> Signed-off-by: Alex Deucher 
>
> Which seems to improperly revert:
> commit d020970959169627d59a711769f8c4b87bf5f90c
> Author: Jaehyun Chung 
> Date:   Tue Aug 24 14:05:48 2021 -0400
>
> drm/amd/display: Add regamma/degamma coefficients and set sRGB
> when TF is BT709
>
> [Why]
> In YUV case, need to set the input TF to sRGB instead of BT709,
> even though the input TF type is distributed. SRGB was not
> being used because pixel format was not being set in the
> surface update sequence.
> Also, we were using the same coefficients for degamma and
> regamma formula, causing the cutoff point of the linear
> section of the curve to be incorrect.
>
> [How]
> Set pixel format in the surface update sequence. Add separate
> coefficient arrays for regamma and degamma.
>
> Reviewed-by: Krunoslav Kovac 
> Acked-by: Mikita Lipski 
> Signed-off-by: Jaehyun Chung 
> Tested-by: Daniel Wheeler 
> Signed-off-by: Alex Deucher 
>
> I think the proper fix is to set it to:
> build_coefficients(, TRANSFER_FUNCTION_SRGB);
>
> Alex
>
> >
> > Signed-off-by: Zeng Heng 
> > ---
> >  drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > index 04f7656906ca..2f807d787c77 100644
> > --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > @@ -1692,7 +1692,7 @@ static void apply_degamma_for_user_regamma(struct 
> > pwl_float_data_ex *rgb_regamma
> > struct pwl_float_data_ex *rgb = rgb_regamma;
> > const struct hw_x_point *coord_x = coordinates_x;
> >
> > -   build_coefficients(, true);
> > +   build_coefficients(, TRANSFER_FUNCTION_BT709);
> >
> > i = 0;
> > while (i != hw_points_num + 1) {
> > --
> > 2.25.1
> >


Re: [PATCH] drm/amdgpu: use dirty framebuffer helper

2022-09-19 Thread Alex Deucher
On Mon, Sep 19, 2022 at 2:44 AM Thomas Zimmermann  wrote:
>
> Hi
>
> Am 06.09.22 um 21:57 schrieb Hamza Mahfooz:
> > Currently, we aren't handling DRM_IOCTL_MODE_DIRTYFB. So, use
> > drm_atomic_helper_dirtyfb() as the dirty callback in the amdgpu_fb_funcs
> > struct.
>
> drm_atomic_helper_dirtyfb() creates a new atomic commit for the
> frambuffer's planes. Drivers can then updates these planes' output
> (e.g., writeback to video memory). I thought that amdgpu simply scans
> out from the framebuffer's memory regions in VRAM. So I'm curious why
> this patch is necessary.

I think in this particular case, the problem is that there are still
some asic which default to non-atomic code which is what is causing
the problem here.  Something like this would fix that:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 5b09c8f4fe95..f5e9dd454c54 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -497,6 +497,11 @@ bool amdgpu_display_ddc_probe(struct
amdgpu_connector *amdgpu_connector,
 static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
.destroy = drm_gem_fb_destroy,
.create_handle = drm_gem_fb_create_handle,
+};
+
+static const struct drm_framebuffer_funcs amdgpu_fb_funcs_atomic = {
+   .destroy = drm_gem_fb_destroy,
+   .create_handle = drm_gem_fb_create_handle,
.dirty = drm_atomic_helper_dirtyfb,
 };

@@ -1102,7 +1107,10 @@ static int
amdgpu_display_gem_fb_verify_and_init(struct drm_device *dev,
if (ret)
goto err;

-   ret = drm_framebuffer_init(dev, >base, _fb_funcs);
+   if (drm_drv_uses_atomic_modeset(adev_to_drm(adev)))
+   ret = drm_framebuffer_init(dev, >base,
_fb_funcs_atomic);
+   else
+   ret = drm_framebuffer_init(dev, >base, _fb_funcs);
if (ret)
goto err;

As for why we need the dirty callback, I think it's used for PSR.

Alex

>
> Best regards
> Thomas
>
> >
> > Signed-off-by: Hamza Mahfooz 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > index c20922a5af9f..5b09c8f4fe95 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > @@ -38,6 +38,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   #include 
> >   #include 
> >   #include 
> > @@ -496,6 +497,7 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector 
> > *amdgpu_connector,
> >   static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
> >   .destroy = drm_gem_fb_destroy,
> >   .create_handle = drm_gem_fb_create_handle,
> > + .dirty = drm_atomic_helper_dirtyfb,
> >   };
> >
> >   uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev


Re: [v2] drm/amd: Add IMU fw version to fw version queries

2022-09-19 Thread Alex Deucher
On Mon, Sep 19, 2022 at 9:42 AM David Francis  wrote:
>
> IMU is a new firmware for GFX11.
>
> There are four means by which firmware version can be queried
> from the driver: device attributes, vf2pf, debugfs,
> and the AMDGPU_INFO_FW_VERSION option in the amdgpu info ioctl.
>
> Add IMU as an option for those four methods.
>
> V2: Added debugfs
>
> CC: Likun Gao 
>
> Signed-off-by: David Francis 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 13 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c   |  4 +++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c|  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  1 +
>  include/uapi/drm/amdgpu_drm.h   |  2 ++
>  5 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 1369c25448dc..56753c3574b1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -328,6 +328,10 @@ static int amdgpu_firmware_info(struct 
> drm_amdgpu_info_firmware *fw_info,
> fw_info->ver = adev->psp.cap_fw_version;
> fw_info->feature = adev->psp.cap_feature_version;
> break;
> +   case AMDGPU_INFO_FW_IMU:
> +   fw_info->ver = adev->gfx.imu_fw_version;
> +   fw_info->feature = 0;
> +   break;
> default:
> return -EINVAL;
> }
> @@ -1488,6 +1492,15 @@ static int amdgpu_debugfs_firmware_info_show(struct 
> seq_file *m, void *unused)
>fw_info.feature, fw_info.ver);
> }
>
> +   /* IMU */
> +   query_fw.fw_type = AMDGPU_INFO_FW_IMU;
> +   query_fw.index = 0;
> +   ret = amdgpu_firmware_info(_info, _fw, adev);
> +   if (ret)
> +   return ret;
> +   seq_printf(m, "IMU feature version: %u, firmware version: 0x%08x\n",
> +  fw_info.feature, fw_info.ver);
> +
> /* PSP SOS */
> query_fw.fw_type = AMDGPU_INFO_FW_SOS;
> ret = amdgpu_firmware_info(_info, _fw, adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index 939c8614f0e3..a576a50fad25 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -630,6 +630,7 @@ FW_VERSION_ATTR(rlc_srlg_fw_version, 0444, 
> gfx.rlc_srlg_fw_version);
>  FW_VERSION_ATTR(rlc_srls_fw_version, 0444, gfx.rlc_srls_fw_version);
>  FW_VERSION_ATTR(mec_fw_version, 0444, gfx.mec_fw_version);
>  FW_VERSION_ATTR(mec2_fw_version, 0444, gfx.mec2_fw_version);
> +FW_VERSION_ATTR(imu_fw_version, 0444, gfx.imu_fw_version);
>  FW_VERSION_ATTR(sos_fw_version, 0444, psp.sos.fw_version);
>  FW_VERSION_ATTR(asd_fw_version, 0444, psp.asd_context.bin_desc.fw_version);
>  FW_VERSION_ATTR(ta_ras_fw_version, 0444, 
> psp.ras_context.context.bin_desc.fw_version);
> @@ -651,7 +652,8 @@ static struct attribute *fw_attrs[] = {
> _attr_ta_ras_fw_version.attr, _attr_ta_xgmi_fw_version.attr,
> _attr_smc_fw_version.attr, _attr_sdma_fw_version.attr,
> _attr_sdma2_fw_version.attr, _attr_vcn_fw_version.attr,
> -   _attr_dmcu_fw_version.attr, NULL
> +   _attr_dmcu_fw_version.attr, _attr_imu_fw_version.attr,
> +   NULL
>  };
>
>  static const struct attribute_group fw_attr_group = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index e4af40b9a8aa..38c46f09d784 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -547,6 +547,7 @@ static void amdgpu_virt_populate_vf2pf_ucode_info(struct 
> amdgpu_device *adev)
> POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_RLC_SRLS, 
> adev->gfx.rlc_srls_fw_version);
> POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_MEC,  
> adev->gfx.mec_fw_version);
> POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_MEC2, 
> adev->gfx.mec2_fw_version);
> +   POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_IMU,  
> adev->gfx.imu_fw_version);
> POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_SOS,  
> adev->psp.sos.fw_version);
> POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_ASD,
> adev->psp.asd_context.bin_desc.fw_version);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> index e78e4c27b62a..6c97148ca0ed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> @@ -70,6 +70,7 @@ enum amd_sriov_ucode_engine_id {
> AMD_SRIOV_UCODE_ID_RLC_SRLS,
> AMD_SRIOV_UCODE_ID_MEC,
> AMD_SRIOV_UCODE_ID_MEC2,
> +   AMD_SRIOV_UCODE_ID_IMU,
> AMD_SRIOV_UCODE_ID_SOS,
> AMD_SRIOV_UCODE_ID_ASD,
> AMD_SRIOV_UCODE_ID_TA_RAS,
> diff --git 

Re: [PATCH] gpu: color: eliminate implicit conversion about enum type

2022-09-19 Thread Alex Deucher
Add the relevant AMD developers to comment.

On Mon, Sep 19, 2022 at 12:05 PM Alex Deucher  wrote:
>
> On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng  wrote:
> >
> > Fix below compile warning when open enum-conversion
> > option check:
> >
> > drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:
> > In function ‘apply_degamma_for_user_regamma’:
> > drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1695:29:
> > error: implicit conversion from ‘enum ’ to ‘enum 
> > dc_transfer_func_predefined’ [-Werror=enum-conversion]
> >  1695 |  build_coefficients(, true);
> >   | ^~~~
> >
> > As 'build_coefficients' definition, it needs enum
> > 'dc_transfer_func_predefined' type acts as the
> > second argument, instead of bool-type one.
> >
> > The numerical values of TRANSFER_FUNCTION_BT709 & true
> > happen to be the same, so there is no change in
> > behavior.
>
> This looks like a regression from:
>
> commit 9b3d76527f6ea50270f7f7ac749493b41783e8bd
> Author: Jaehyun Chung 
> Date:   Mon Aug 30 16:46:42 2021 -0400
>
> drm/amd/display: Revert adding degamma coefficients
>
> [Why]
> Degamma coefficients are calculated in our degamma formula using
> the regamma coefficients. We do not need to add separate degamma
> coefficients.
>
> [How]
> Remove the change to add separate degamma coefficients.
>
> Reviewed-by: Krunoslav Kovac 
> Acked-by: Mikita Lipski 
> Signed-off-by: Jaehyun Chung 
> Tested-by: Daniel Wheeler 
> Signed-off-by: Alex Deucher 
>
> Which seems to improperly revert:
> commit d020970959169627d59a711769f8c4b87bf5f90c
> Author: Jaehyun Chung 
> Date:   Tue Aug 24 14:05:48 2021 -0400
>
> drm/amd/display: Add regamma/degamma coefficients and set sRGB
> when TF is BT709
>
> [Why]
> In YUV case, need to set the input TF to sRGB instead of BT709,
> even though the input TF type is distributed. SRGB was not
> being used because pixel format was not being set in the
> surface update sequence.
> Also, we were using the same coefficients for degamma and
> regamma formula, causing the cutoff point of the linear
> section of the curve to be incorrect.
>
> [How]
> Set pixel format in the surface update sequence. Add separate
> coefficient arrays for regamma and degamma.
>
> Reviewed-by: Krunoslav Kovac 
> Acked-by: Mikita Lipski 
> Signed-off-by: Jaehyun Chung 
> Tested-by: Daniel Wheeler 
> Signed-off-by: Alex Deucher 
>
> I think the proper fix is to set it to:
> build_coefficients(, TRANSFER_FUNCTION_SRGB);
>
> Alex
>
> >
> > Signed-off-by: Zeng Heng 
> > ---
> >  drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c 
> > b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > index 04f7656906ca..2f807d787c77 100644
> > --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > @@ -1692,7 +1692,7 @@ static void apply_degamma_for_user_regamma(struct 
> > pwl_float_data_ex *rgb_regamma
> > struct pwl_float_data_ex *rgb = rgb_regamma;
> > const struct hw_x_point *coord_x = coordinates_x;
> >
> > -   build_coefficients(, true);
> > +   build_coefficients(, TRANSFER_FUNCTION_BT709);
> >
> > i = 0;
> > while (i != hw_points_num + 1) {
> > --
> > 2.25.1
> >


Re: [PATCH] gpu: color: eliminate implicit conversion about enum type

2022-09-19 Thread Alex Deucher
On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng  wrote:
>
> Fix below compile warning when open enum-conversion
> option check:
>
> drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:
> In function ‘apply_degamma_for_user_regamma’:
> drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1695:29:
> error: implicit conversion from ‘enum ’ to ‘enum 
> dc_transfer_func_predefined’ [-Werror=enum-conversion]
>  1695 |  build_coefficients(, true);
>   | ^~~~
>
> As 'build_coefficients' definition, it needs enum
> 'dc_transfer_func_predefined' type acts as the
> second argument, instead of bool-type one.
>
> The numerical values of TRANSFER_FUNCTION_BT709 & true
> happen to be the same, so there is no change in
> behavior.

This looks like a regression from:

commit 9b3d76527f6ea50270f7f7ac749493b41783e8bd
Author: Jaehyun Chung 
Date:   Mon Aug 30 16:46:42 2021 -0400

drm/amd/display: Revert adding degamma coefficients

[Why]
Degamma coefficients are calculated in our degamma formula using
the regamma coefficients. We do not need to add separate degamma
coefficients.

[How]
Remove the change to add separate degamma coefficients.

Reviewed-by: Krunoslav Kovac 
Acked-by: Mikita Lipski 
Signed-off-by: Jaehyun Chung 
Tested-by: Daniel Wheeler 
Signed-off-by: Alex Deucher 

Which seems to improperly revert:
commit d020970959169627d59a711769f8c4b87bf5f90c
Author: Jaehyun Chung 
Date:   Tue Aug 24 14:05:48 2021 -0400

drm/amd/display: Add regamma/degamma coefficients and set sRGB
when TF is BT709

[Why]
In YUV case, need to set the input TF to sRGB instead of BT709,
even though the input TF type is distributed. SRGB was not
being used because pixel format was not being set in the
surface update sequence.
Also, we were using the same coefficients for degamma and
regamma formula, causing the cutoff point of the linear
section of the curve to be incorrect.

[How]
Set pixel format in the surface update sequence. Add separate
coefficient arrays for regamma and degamma.

Reviewed-by: Krunoslav Kovac 
Acked-by: Mikita Lipski 
Signed-off-by: Jaehyun Chung 
Tested-by: Daniel Wheeler 
Signed-off-by: Alex Deucher 

I think the proper fix is to set it to:
build_coefficients(, TRANSFER_FUNCTION_SRGB);

Alex

>
> Signed-off-by: Zeng Heng 
> ---
>  drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c 
> b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> index 04f7656906ca..2f807d787c77 100644
> --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> @@ -1692,7 +1692,7 @@ static void apply_degamma_for_user_regamma(struct 
> pwl_float_data_ex *rgb_regamma
> struct pwl_float_data_ex *rgb = rgb_regamma;
> const struct hw_x_point *coord_x = coordinates_x;
>
> -   build_coefficients(, true);
> +   build_coefficients(, TRANSFER_FUNCTION_BT709);
>
> i = 0;
> while (i != hw_points_num + 1) {
> --
> 2.25.1
>


Re: [PATCH] drm/amdgpu: getting fan speed pwm for vega10 properly

2022-09-19 Thread Alex Deucher
Applied.  Thanks!

Alex

On Sun, Sep 18, 2022 at 10:06 PM Quan, Evan  wrote:
>
> [AMD Official Use Only - General]
>
> Reviewed-by: Evan Quan 
>
> > -Original Message-
> > From: amd-gfx  On Behalf Of
> > stalkerg
> > Sent: Sunday, September 18, 2022 3:23 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander ; stalkerg
> > 
> > Subject: [PATCH] drm/amdgpu: getting fan speed pwm for vega10 properly
> >
> > Instead of using RPM speed, we will use a function from vega20 based on
> > PWM registers.
> >
> > Signed-off-by: Yury Zhuravlev 
> > ---
> >  .../amd/pm/powerplay/hwmgr/vega10_thermal.c   | 25 +--
> >  1 file changed, 12 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> > index dad3e3741a4e..190af79f3236 100644
> > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> > @@ -67,22 +67,21 @@ int vega10_fan_ctrl_get_fan_speed_info(struct
> > pp_hwmgr *hwmgr,
> >  int vega10_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
> >   uint32_t *speed)
> >  {
> > - uint32_t current_rpm;
> > - uint32_t percent = 0;
> > -
> > - if (hwmgr->thermal_controller.fanInfo.bNoFan)
> > - return 0;
> > + struct amdgpu_device *adev = hwmgr->adev;
> > + uint32_t duty100, duty;
> > + uint64_t tmp64;
> >
> > - if (vega10_get_current_rpm(hwmgr, _rpm))
> > - return -1;
> > + duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0,
> > mmCG_FDO_CTRL1),
> > + CG_FDO_CTRL1, FMAX_DUTY100);
> > + duty = REG_GET_FIELD(RREG32_SOC15(THM, 0,
> > mmCG_THERMAL_STATUS),
> > + CG_THERMAL_STATUS, FDO_PWM_DUTY);
> >
> > - if (hwmgr->thermal_controller.
> > - advanceFanControlParameters.usMaxFanRPM != 0)
> > - percent = current_rpm * 255 /
> > - hwmgr->thermal_controller.
> > - advanceFanControlParameters.usMaxFanRPM;
> > + if (!duty100)
> > + return -EINVAL;
> >
> > - *speed = MIN(percent, 255);
> > + tmp64 = (uint64_t)duty * 255;
> > + do_div(tmp64, duty100);
> > + *speed = MIN((uint32_t)tmp64, 255);
> >
> >   return 0;
> >  }
> > --
> > 2.35.1


Re: [PATCH 1/2] drm/amdgpu: fix deadlock caused by overflow

2022-09-19 Thread Andrey Grodzovsky
I don't know if issue still exist but it worth checking with Christian 
who wrote this patch.


Andrey


On 2022-09-16 23:31, Zhao, Victor wrote:

[AMD Official Use Only - General]

Hi Andrey,

Yes, moving irq disable can fix the issue. Change in amdgpu_fence_process is 
just want to make sure driver can correct itself from an overflow situation. 
Didn’t know about the previous issue there.

Do you know if the issue still exists? Or is it on VCE only?


Thanks,
Victor



-Original Message-
From: Grodzovsky, Andrey 
Sent: Friday, September 16, 2022 9:50 PM
To: Koenig, Christian ; Zhao, Victor 
; amd-gfx@lists.freedesktop.org
Cc: Deng, Emily 
Subject: Re: [PATCH 1/2] drm/amdgpu: fix deadlock caused by overflow


On 2022-09-16 01:18, Christian König wrote:

Am 15.09.22 um 22:37 schrieb Andrey Grodzovsky:

On 2022-09-15 15:26, Christian König wrote:

Am 15.09.22 um 20:29 schrieb Andrey Grodzovsky:

On 2022-09-15 06:09, Zhao, Victor wrote:

[AMD Official Use Only - General]

Hi Christian,

The test sequence is executing a compute engine hang while running
a lot of containers submitting gfx jobs. We have advanced tdr mode
and mode2 reset enabled on driver.
When a compute hang job timeout happens, the 2 jobs on the gfx
pending list maybe signaled after drm_sched_stop. So they will not
be removed from pending list but have the
DMA_FENCE_FLAG_SIGNALED_BIT set.
At the amdgpu_device_recheck_guilty_jobs step, the first job will
be rerun and removed from pending list.
At the resubmit setp, the second job (with signaled bit) will be
resubmitted. Since it still has signaled bit, drm_sched_job_done
will be called directly. This decrease the hw_rq_count which
allows more jobs emitted but did not clean fence_drv rcu ptr.
This results in an overflow in the fence_drv. Since we will use
num_fences_mask in amdgpu_fence_process, when overflow happens,
the signal of some job will be skipped which result in an infinite
wait for the fence_drv rcu ptr.

So close irq before sched_stop could avoid signal jobs after
drm_sched_stop. And signal job one by one in fence_process instead
of using a mask will handle the overflow situation.

Another fix could be skip submitting jobs which already signaled
during resubmit stage, which may look cleaner.

Please help give some advice.


How about the code bellow  instead ? The real problem is that we
reuse a dma fence twice which is not according to fma fence design,
so maybe this can help ?


diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 8adeb7469f1e..033f0ae16784 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -164,6 +164,10 @@ int amdgpu_fence_emit(struct amdgpu_ring
*ring, struct dma_fence **f, struct amd
     if (job && job->job_run_counter) {
     /* reinit seq for resubmitted jobs */
     fence->seqno = seq;
+
+   /* For resubmitted job clear the singled bit */
+   celar_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT,
>flags);
+

Upstream will pretty much kill you for that.

Re-setting a fence from a signaled to an unsignaled state is a
massive no-go.

Christian.


Is it worse then doing fence->seqno = seq; ? This is already a huge
hack , no ?

No, it's as equally bad. I don't think we can do either.

Christian.


And all those ugly hack are there because we reuse a dma_fence (hw_fence 
embedded into the job) and correct me if I am wrong but I don't think dma_fence 
is ever supposed to be reused.

So maybe like Victor suggested we should move close and flush irq before 
sched_stop - this in my opinion should solve the issue, but Victor - why then 
you still need the change in amdgpu_fence_process ? You will not have the 
overflow situation because by moving irq_disable before stop any job that 
signaled will be removed from the scheduler pending list anyway. Also not that 
this change reverts 'drm/amdgpu: sanitize fence numbers' and could reintroduce 
that bug.

Andrey



Andrey



     /* TO be inline with external fence creation and
other drivers */
     dma_fence_get(fence);
     } else {


Andrey




Thanks,
Victor



-Original Message-
From: Koenig, Christian 
Sent: Thursday, September 15, 2022 2:32 PM
To: Zhao, Victor ;
amd-gfx@lists.freedesktop.org; Grodzovsky, Andrey

Cc: Deng, Emily 
Subject: Re: [PATCH 1/2] drm/amdgpu: fix deadlock caused by
overflow



Am 15.09.22 um 06:02 schrieb Zhao, Victor:

[AMD Official Use Only - General]

Ping.

Hi @Koenig, Christian and @Grodzovsky, Andrey,

We found some reset related issues during stress test on the
sequence. Please help give some comments.


Thanks,
Victor



-Original Message-
From: Victor Zhao 
Sent: Wednesday, September 14, 2022 6:10 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deng, Emily ; Grodzovsky, Andrey
; Zhao, Victor 
Subject: [PATCH 1/2] drm/amdgpu: fix deadlock caused by overflow

[background]
For a gpu recovery caused 

Re: [PATCH V2 4/7] drm/amd/pm: move SMU13.0.0 related pptable settings to smu_v13_0_0_ppt.c

2022-09-19 Thread Alex Deucher
On Mon, Sep 19, 2022 at 9:48 AM Alex Deucher  wrote:
>
> On Sun, Sep 18, 2022 at 10:04 PM Evan Quan  wrote:
> >
> > Separate those ASIC specific settings from common helpers.
> >
> > Signed-off-by: Evan Quan 
> > Change-Id: Ie3224b8719d48c6e6936b738df379959bd1df4ad
> > ---
> >  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c|  1 -
> >  .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 16 +---
> >  2 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
> > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > index 610f9b68ef73..7adbdd3cc13b 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > @@ -1027,7 +1027,6 @@ static void smu_interrupt_work_fn(struct work_struct 
> > *work)
> >  static void smu_pptable_source_init(struct smu_context *smu)
> >  {
> > switch (smu->adev->ip_versions[MP1_HWIP][0]) {
> > -   case IP_VERSION(13, 0, 0):
>
> Is this change intended?

Nevermind, I missed your response on the original thread.

Alex


>
> Alex
>
> > case IP_VERSION(13, 0, 7):
> > smu->pptable_source = PPTABLE_SOURCE_PMFW;
> > smu->pptable_id = smu->smu_table.boot_values.pp_table_id;
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c 
> > b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> > index 1d454485e0d9..fd405e2420cd 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> > @@ -377,9 +377,12 @@ static int smu_v13_0_0_setup_pptable(struct 
> > smu_context *smu)
> > struct amdgpu_device *adev = smu->adev;
> > int ret = 0;
> >
> > -   ret = smu_v13_0_0_get_pptable_from_pmfw(smu,
> > -   
> > _table->power_play_table,
> > -   
> > _table->power_play_table_size);
> > +   if (smu->pptable_source == PPTABLE_SOURCE_PMFW)
> > +   ret = smu_v13_0_0_get_pptable_from_pmfw(smu,
> > +   
> > _table->power_play_table,
> > +   
> > _table->power_play_table_size);
> > +   else
> > +   ret = smu_v13_0_setup_pptable(smu);
> > if (ret)
> > return ret;
> >
> > @@ -1753,6 +1756,12 @@ static int smu_v13_0_0_set_mp1_state(struct 
> > smu_context *smu,
> > return ret;
> >  }
> >
> > +static void smu_v13_0_0_pptable_source_init(struct smu_context *smu)
> > +{
> > +   smu->pptable_source = PPTABLE_SOURCE_PMFW;
> > +   smu->pptable_id = smu->smu_table.boot_values.pp_table_id;
> > +}
> > +
> >  static const struct pptable_funcs smu_v13_0_0_ppt_funcs = {
> > .get_allowed_feature_mask = smu_v13_0_0_get_allowed_feature_mask,
> > .set_default_dpm_table = smu_v13_0_0_set_default_dpm_table,
> > @@ -1822,6 +1831,7 @@ static const struct pptable_funcs 
> > smu_v13_0_0_ppt_funcs = {
> > .mode1_reset_is_support = smu_v13_0_0_is_mode1_reset_supported,
> > .mode1_reset = smu_v13_0_mode1_reset,
> > .set_mp1_state = smu_v13_0_0_set_mp1_state,
> > +   .pptable_source_init = smu_v13_0_0_pptable_source_init,
> >  };
> >
> >  void smu_v13_0_0_set_ppt_funcs(struct smu_context *smu)
> > --
> > 2.34.1
> >


Re: [PATCH V2 4/7] drm/amd/pm: move SMU13.0.0 related pptable settings to smu_v13_0_0_ppt.c

2022-09-19 Thread Alex Deucher
On Sun, Sep 18, 2022 at 10:04 PM Evan Quan  wrote:
>
> Separate those ASIC specific settings from common helpers.
>
> Signed-off-by: Evan Quan 
> Change-Id: Ie3224b8719d48c6e6936b738df379959bd1df4ad
> ---
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c|  1 -
>  .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 16 +---
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 610f9b68ef73..7adbdd3cc13b 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1027,7 +1027,6 @@ static void smu_interrupt_work_fn(struct work_struct 
> *work)
>  static void smu_pptable_source_init(struct smu_context *smu)
>  {
> switch (smu->adev->ip_versions[MP1_HWIP][0]) {
> -   case IP_VERSION(13, 0, 0):

Is this change intended?

Alex

> case IP_VERSION(13, 0, 7):
> smu->pptable_source = PPTABLE_SOURCE_PMFW;
> smu->pptable_id = smu->smu_table.boot_values.pp_table_id;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index 1d454485e0d9..fd405e2420cd 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -377,9 +377,12 @@ static int smu_v13_0_0_setup_pptable(struct smu_context 
> *smu)
> struct amdgpu_device *adev = smu->adev;
> int ret = 0;
>
> -   ret = smu_v13_0_0_get_pptable_from_pmfw(smu,
> -   _table->power_play_table,
> -   
> _table->power_play_table_size);
> +   if (smu->pptable_source == PPTABLE_SOURCE_PMFW)
> +   ret = smu_v13_0_0_get_pptable_from_pmfw(smu,
> +   
> _table->power_play_table,
> +   
> _table->power_play_table_size);
> +   else
> +   ret = smu_v13_0_setup_pptable(smu);
> if (ret)
> return ret;
>
> @@ -1753,6 +1756,12 @@ static int smu_v13_0_0_set_mp1_state(struct 
> smu_context *smu,
> return ret;
>  }
>
> +static void smu_v13_0_0_pptable_source_init(struct smu_context *smu)
> +{
> +   smu->pptable_source = PPTABLE_SOURCE_PMFW;
> +   smu->pptable_id = smu->smu_table.boot_values.pp_table_id;
> +}
> +
>  static const struct pptable_funcs smu_v13_0_0_ppt_funcs = {
> .get_allowed_feature_mask = smu_v13_0_0_get_allowed_feature_mask,
> .set_default_dpm_table = smu_v13_0_0_set_default_dpm_table,
> @@ -1822,6 +1831,7 @@ static const struct pptable_funcs smu_v13_0_0_ppt_funcs 
> = {
> .mode1_reset_is_support = smu_v13_0_0_is_mode1_reset_supported,
> .mode1_reset = smu_v13_0_mode1_reset,
> .set_mp1_state = smu_v13_0_0_set_mp1_state,
> +   .pptable_source_init = smu_v13_0_0_pptable_source_init,
>  };
>
>  void smu_v13_0_0_set_ppt_funcs(struct smu_context *smu)
> --
> 2.34.1
>


[v2] drm/amd: Add IMU fw version to fw version queries

2022-09-19 Thread David Francis
IMU is a new firmware for GFX11.

There are four means by which firmware version can be queried
from the driver: device attributes, vf2pf, debugfs,
and the AMDGPU_INFO_FW_VERSION option in the amdgpu info ioctl.

Add IMU as an option for those four methods.

V2: Added debugfs

CC: Likun Gao 

Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c   |  4 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c|  1 +
 drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  1 +
 include/uapi/drm/amdgpu_drm.h   |  2 ++
 5 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 1369c25448dc..56753c3574b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -328,6 +328,10 @@ static int amdgpu_firmware_info(struct 
drm_amdgpu_info_firmware *fw_info,
fw_info->ver = adev->psp.cap_fw_version;
fw_info->feature = adev->psp.cap_feature_version;
break;
+   case AMDGPU_INFO_FW_IMU:
+   fw_info->ver = adev->gfx.imu_fw_version;
+   fw_info->feature = 0;
+   break;
default:
return -EINVAL;
}
@@ -1488,6 +1492,15 @@ static int amdgpu_debugfs_firmware_info_show(struct 
seq_file *m, void *unused)
   fw_info.feature, fw_info.ver);
}
 
+   /* IMU */
+   query_fw.fw_type = AMDGPU_INFO_FW_IMU;
+   query_fw.index = 0;
+   ret = amdgpu_firmware_info(_info, _fw, adev);
+   if (ret)
+   return ret;
+   seq_printf(m, "IMU feature version: %u, firmware version: 0x%08x\n",
+  fw_info.feature, fw_info.ver);
+
/* PSP SOS */
query_fw.fw_type = AMDGPU_INFO_FW_SOS;
ret = amdgpu_firmware_info(_info, _fw, adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 939c8614f0e3..a576a50fad25 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -630,6 +630,7 @@ FW_VERSION_ATTR(rlc_srlg_fw_version, 0444, 
gfx.rlc_srlg_fw_version);
 FW_VERSION_ATTR(rlc_srls_fw_version, 0444, gfx.rlc_srls_fw_version);
 FW_VERSION_ATTR(mec_fw_version, 0444, gfx.mec_fw_version);
 FW_VERSION_ATTR(mec2_fw_version, 0444, gfx.mec2_fw_version);
+FW_VERSION_ATTR(imu_fw_version, 0444, gfx.imu_fw_version);
 FW_VERSION_ATTR(sos_fw_version, 0444, psp.sos.fw_version);
 FW_VERSION_ATTR(asd_fw_version, 0444, psp.asd_context.bin_desc.fw_version);
 FW_VERSION_ATTR(ta_ras_fw_version, 0444, 
psp.ras_context.context.bin_desc.fw_version);
@@ -651,7 +652,8 @@ static struct attribute *fw_attrs[] = {
_attr_ta_ras_fw_version.attr, _attr_ta_xgmi_fw_version.attr,
_attr_smc_fw_version.attr, _attr_sdma_fw_version.attr,
_attr_sdma2_fw_version.attr, _attr_vcn_fw_version.attr,
-   _attr_dmcu_fw_version.attr, NULL
+   _attr_dmcu_fw_version.attr, _attr_imu_fw_version.attr,
+   NULL
 };
 
 static const struct attribute_group fw_attr_group = {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index e4af40b9a8aa..38c46f09d784 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -547,6 +547,7 @@ static void amdgpu_virt_populate_vf2pf_ucode_info(struct 
amdgpu_device *adev)
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_RLC_SRLS, 
adev->gfx.rlc_srls_fw_version);
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_MEC,  
adev->gfx.mec_fw_version);
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_MEC2, 
adev->gfx.mec2_fw_version);
+   POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_IMU,  
adev->gfx.imu_fw_version);
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_SOS,  
adev->psp.sos.fw_version);
POPULATE_UCODE_INFO(vf2pf_info, AMD_SRIOV_UCODE_ID_ASD,
adev->psp.asd_context.bin_desc.fw_version);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h 
b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
index e78e4c27b62a..6c97148ca0ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -70,6 +70,7 @@ enum amd_sriov_ucode_engine_id {
AMD_SRIOV_UCODE_ID_RLC_SRLS,
AMD_SRIOV_UCODE_ID_MEC,
AMD_SRIOV_UCODE_ID_MEC2,
+   AMD_SRIOV_UCODE_ID_IMU,
AMD_SRIOV_UCODE_ID_SOS,
AMD_SRIOV_UCODE_ID_ASD,
AMD_SRIOV_UCODE_ID_TA_RAS,
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index c2c9c674a223..3e00cec47f52 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -755,6 +755,8 @@ struct drm_amdgpu_cs_chunk_data {
#define AMDGPU_INFO_FW_TOC  0x15
/* Subquery id: 

Re: [PATCH] gpu: dc: fix enum conversion in display_mode_vba

2022-09-19 Thread Zeng Heng

the correct prefix is "drm/amdgpu: "

Got it, and I would notice that point at the next letter.


在 2022/9/19 15:44, Christian König 写道:

Am 19.09.22 um 03:41 schrieb Zeng Heng:

Fix below compile warning when open enum-conversion
option check (compiled with -Wenum-conversion):

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c: 


In function ‘dml20_ModeSupportAndSystemConfigurationFull’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3900:44: 

error: implicit conversion from ‘enum ’ to ‘enum 
odm_combine_mode’ [-Werror=enum-conversion]

  3900 | locals->ODMCombineEnablePerState[i][k] = false;
   |    ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3904:46: 

error: implicit conversion from ‘enum ’ to ‘enum 
odm_combine_mode’ [-Werror=enum-conversion]

  3904 |   locals->ODMCombineEnablePerState[i][k] = true;
   |  ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3907:46: 

error: implicit conversion from ‘enum ’ to ‘enum 
odm_combine_mode’ [-Werror=enum-conversion]

  3907 |   locals->ODMCombineEnablePerState[i][k] = true;
   |  ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3960:45: 

error: implicit conversion from ‘enum ’ to ‘enum 
odm_combine_mode’ [-Werror=enum-conversion]

  3960 |  locals->ODMCombineEnablePerState[i][k] = false;

Use the proper value from the right enumerated type,
dm_odm_combine_mode_disabled & dm_odm_combine_mode_2to1,
so there is no more implicit conversion.

The numerical values of dm_odm_combine_mode_disabled
& false and dm_odm_combine_mode_2to1 & true
happen to be the same, so there is no change in
behavior.


In the subject line the correct prefix is "drm/amdgpu: ", but 
apart from that looks good to me as well.


But our DC team has to take a closer look.

Thanks,
Christian.



Signed-off-by: Zeng Heng 
---
  .../amd/display/dc/dml/dcn20/display_mode_vba_20.c   |  8 
  .../amd/display/dc/dml/dcn20/display_mode_vba_20v2.c | 10 +-
  .../amd/display/dc/dml/dcn21/display_mode_vba_21.c   | 12 ++--
  3 files changed, 15 insertions(+), 15 deletions(-)

diff --git 
a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c

index d3b5b6fedf04..6266b0788387 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
@@ -3897,14 +3897,14 @@ void 
dml20_ModeSupportAndSystemConfigurationFull(struct display_mode_lib 
*mode_l
mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine = 
mode_lib->vba.PixelClock[k] / 2
  * (1 + 
mode_lib->vba.DISPCLKDPPCLKDSCCLKDownSpreading / 100.0);

  -    locals->ODMCombineEnablePerState[i][k] = false;
+    locals->ODMCombineEnablePerState[i][k] = 
dm_odm_combine_mode_disabled;
  mode_lib->vba.PlaneRequiredDISPCLK = 
mode_lib->vba.PlaneRequiredDISPCLKWithoutODMCombine;

  if (mode_lib->vba.ODMCapability) {
  if 
(locals->PlaneRequiredDISPCLKWithoutODMCombine > 
mode_lib->vba.MaxDispclkRoundedDownToDFSGranularity) {

- locals->ODMCombineEnablePerState[i][k] = true;
+ locals->ODMCombineEnablePerState[i][k] = dm_odm_combine_mode_2to1;
  mode_lib->vba.PlaneRequiredDISPCLK = 
mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine;
  } else if (locals->HActive[k] > 
DCN20_MAX_420_IMAGE_WIDTH && locals->OutputFormat[k] == dm_420) {

- locals->ODMCombineEnablePerState[i][k] = true;
+ locals->ODMCombineEnablePerState[i][k] = dm_odm_combine_mode_2to1;
  mode_lib->vba.PlaneRequiredDISPCLK = 
mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine;

  }
  }
@@ -3957,7 +3957,7 @@ void 
dml20_ModeSupportAndSystemConfigurationFull(struct display_mode_lib 
*mode_l

  locals->RequiredDISPCLK[i][j] = 0.0;
  locals->DISPCLK_DPPCLK_Support[i][j] = true;
  for (k = 0; k <= mode_lib->vba.NumberOfActivePlanes 
- 1; k++) {

-    locals->ODMCombineEnablePerState[i][k] = false;
+    locals->ODMCombineEnablePerState[i][k] = 
dm_odm_combine_mode_disabled;
  if (locals->SwathWidthYSingleDPP[k] <= 
locals->MaximumSwathWidth[k]) {

  locals->NoOfDPP[i][j][k] = 1;
  locals->RequiredDPPCLK[i][j][k] = 
locals->MinDPPCLKUsingSingleDPP[k]
diff --git 
a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c

index edd098c7eb92..989d83ee3842 100644
--- 

[PATCH] gpu: color: eliminate implicit conversion about enum type

2022-09-19 Thread Zeng Heng
Fix below compile warning when open enum-conversion
option check:

drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:
In function ‘apply_degamma_for_user_regamma’:
drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1695:29:
error: implicit conversion from ‘enum ’ to ‘enum 
dc_transfer_func_predefined’ [-Werror=enum-conversion]
 1695 |  build_coefficients(, true);
  | ^~~~

As 'build_coefficients' definition, it needs enum
'dc_transfer_func_predefined' type acts as the
second argument, instead of bool-type one.

The numerical values of TRANSFER_FUNCTION_BT709 & true
happen to be the same, so there is no change in
behavior.

Signed-off-by: Zeng Heng 
---
 drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c 
b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
index 04f7656906ca..2f807d787c77 100644
--- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
+++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
@@ -1692,7 +1692,7 @@ static void apply_degamma_for_user_regamma(struct 
pwl_float_data_ex *rgb_regamma
struct pwl_float_data_ex *rgb = rgb_regamma;
const struct hw_x_point *coord_x = coordinates_x;
 
-   build_coefficients(, true);
+   build_coefficients(, TRANSFER_FUNCTION_BT709);
 
i = 0;
while (i != hw_points_num + 1) {
-- 
2.25.1



Re: mainline build failure (new) for x86_64 allmodconfig with clang

2022-09-19 Thread Sudip Mukherjee
Hi Nathan,

On Sat, Sep 17, 2022 at 2:25 PM Nathan Chancellor  wrote:
>
> Hi Sudip,
>
> On Sat, Sep 17, 2022 at 11:55:05AM +0100, Sudip Mukherjee (Codethink) wrote:
> > Hi All,
> >
> > The latest mainline kernel branch fails to build for x86_64 allmodconfig
> > with clang. The errors are:
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn314/display_mode_vba_314.c:4020:6:
> >  error: stack frame size (2184) exceeds limit (2048) in 
> > 'dml314_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
> > void dml314_ModeSupportAndSystemConfigurationFull(struct display_mode_lib 
> > *mode_lib)
> >  ^
> > 1 error generated.
> >
> >
> > Note: This is a new error seen on top on a335366bad13 ("Merge tag 
> > 'gpio-fixes-for-v6.0-rc6' of 
> > git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux").
> > Previous reported clang build error is now fixed, thanks to Nathan.
> >
> > And, it appears Nathan has already sent a fix for this:
> > https://github.com/intel-lab-lkp/linux/commit/4ecc45d7585ae2e05d622879ad97e13a7d8c595b
> > https://github.com/intel-lab-lkp/linux/commit/819976a950b497d7f10cd9a198a94c26a9005b30
>
> I did not realize this was a mainline issue too :( it seems that
> commit af2f2a256e04 ("drm/amd/display: Enable dlg and vba compilation
> for dcn314") is needed to see this and it was only in -next for three
> releases (20220914 to 20220916), which I missed checking as closely as I
> normally do due to Plumbers wrapping up and traveling.

This has become a mainline issue since today.
I saw in it linux-next also, but missed reporting :(


-- 
Regards
Sudip


Re: [PATCH] gpu: color: eliminate implicit conversion about enum type

2022-09-19 Thread Wei Yongjun
gpu: color: fix enum-conversion compile warning

标题我记得不跟你所过让你改的


On 2022/9/19 9:44, Zeng Heng wrote:
> Fix below compile warning when open enum-conversion
> option check:
> 
> drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:
> In function ‘apply_degamma_for_user_regamma’:
> drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1695:29:
> error: implicit conversion from ‘enum ’ to ‘enum 
> dc_transfer_func_predefined’ [-Werror=enum-conversion]
>  1695 |  build_coefficients(, true);
>   | ^~~~
> 
> As 'build_coefficients' definition, it needs enum
> 'dc_transfer_func_predefined' type acts as the
> second argument, instead of bool-type one.
> 
> The numerical values of TRANSFER_FUNCTION_BT709 & true
> happen to be the same, so there is no change in
> behavior.
> 
> Signed-off-by: Zeng Heng 
> ---
>  drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c 
> b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> index 04f7656906ca..2f807d787c77 100644
> --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> @@ -1692,7 +1692,7 @@ static void apply_degamma_for_user_regamma(struct 
> pwl_float_data_ex *rgb_regamma
>   struct pwl_float_data_ex *rgb = rgb_regamma;
>   const struct hw_x_point *coord_x = coordinates_x;
>  
> - build_coefficients(, true);
> + build_coefficients(, TRANSFER_FUNCTION_BT709);
>  
>   i = 0;
>   while (i != hw_points_num + 1) {
> 


Re: [PATCH 1/2] drm/amd/display: Reduce number of arguments of dml314's CalculateWatermarksAndDRAMSpeedChangeSupport()

2022-09-19 Thread Tom Rix



On 9/16/22 2:06 PM, Nathan Chancellor wrote:

Most of the arguments are identical between the two call sites and they
can be accessed through the 'struct vba_vars_st' pointer. This reduces
the total amount of stack space that
dml314_ModeSupportAndSystemConfigurationFull() uses by 240 bytes with
LLVM 16 (2216 -> 1976), helping clear up the following clang warning:

   
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn314/display_mode_vba_314.c:4020:6:
 error: stack frame size (2216) exceeds limit (2048) in 
'dml314_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
   void dml314_ModeSupportAndSystemConfigurationFull(struct display_mode_lib 
*mode_lib)
^
   1 error generated.

Link: https://github.com/ClangBuiltLinux/linux/issues/1710
Reported-by: "kernelci.org bot" 
Signed-off-by: Nathan Chancellor 


Nathan,

I like this change but I don't think it goes far enough.

There are many similar functions in this file and there other 
display_node_vba_*.c files that pass too many vba_vars_st elements.


I think most/all of the static functions should be refactored to pass 
vba_vars_st * or vba_vars_st **


fwiw, i found the calling function 
DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation, 
hilariously long :)


I'll do the change if you want to pass this to me, I promise not to add 
to the above function name.


Tom


---

This is just commit ab2ac59c32db ("drm/amd/display: Reduce number of
arguments of dml31's CalculateWatermarksAndDRAMSpeedChangeSupport()")
applied to dml314.

  .../dc/dml/dcn314/display_mode_vba_314.c  | 248 --
  1 file changed, 52 insertions(+), 196 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
index 2829f179f982..32ceb72f7a14 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
@@ -325,64 +325,28 @@ static void CalculateVupdateAndDynamicMetadataParameters(
  static void CalculateWatermarksAndDRAMSpeedChangeSupport(
struct display_mode_lib *mode_lib,
unsigned int PrefetchMode,
-   unsigned int NumberOfActivePlanes,
-   unsigned int MaxLineBufferLines,
-   unsigned int LineBufferSize,
-   unsigned int WritebackInterfaceBufferSize,
double DCFCLK,
double ReturnBW,
-   bool SynchronizedVBlank,
-   unsigned int dpte_group_bytes[],
-   unsigned int MetaChunkSize,
double UrgentLatency,
double ExtraLatency,
-   double WritebackLatency,
-   double WritebackChunkSize,
double SOCCLK,
-   double DRAMClockChangeLatency,
-   double SRExitTime,
-   double SREnterPlusExitTime,
-   double SRExitZ8Time,
-   double SREnterPlusExitZ8Time,
double DCFCLKDeepSleep,
unsigned int DETBufferSizeY[],
unsigned int DETBufferSizeC[],
unsigned int SwathHeightY[],
unsigned int SwathHeightC[],
-   unsigned int LBBitPerPixel[],
double SwathWidthY[],
double SwathWidthC[],
-   double HRatio[],
-   double HRatioChroma[],
-   unsigned int vtaps[],
-   unsigned int VTAPsChroma[],
-   double VRatio[],
-   double VRatioChroma[],
-   unsigned int HTotal[],
-   double PixelClock[],
-   unsigned int BlendingAndTiming[],
unsigned int DPPPerPlane[],
double BytePerPixelDETY[],
double BytePerPixelDETC[],
-   double DSTXAfterScaler[],
-   double DSTYAfterScaler[],
-   bool WritebackEnable[],
-   enum source_format_class WritebackPixelFormat[],
-   double WritebackDestinationWidth[],
-   double WritebackDestinationHeight[],
-   double WritebackSourceHeight[],
bool UnboundedRequestEnabled,
unsigned int CompressedBufferSizeInkByte,
enum clock_change_support *DRAMClockChangeSupport,
-   double *UrgentWatermark,
-   double *WritebackUrgentWatermark,
-   double *DRAMClockChangeWatermark,
-   double *WritebackDRAMClockChangeWatermark,
double *StutterExitWatermark,
double *StutterEnterPlusExitWatermark,
double *Z8StutterExitWatermark,
-   double *Z8StutterEnterPlusExitWatermark,
-   double *MinActiveDRAMClockChangeLatencySupported);
+   double *Z8StutterEnterPlusExitWatermark);
  
  static void CalculateDCFCLKDeepSleep(

struct 

[PATCH] drm/amd/display: remove redundant CalculateTWait's

2022-09-19 Thread Tom Rix
There are several copies of CalculateTwait.
Reduce to one instance and change local variable name to match common usage.

Signed-off-by: Tom Rix 
---
 .../dc/dml/dcn20/display_mode_vba_20.c| 16 +++---
 .../dc/dml/dcn20/display_mode_vba_20v2.c  | 21 ++-
 .../dc/dml/dcn21/display_mode_vba_21.c| 19 +
 .../dc/dml/dcn30/display_mode_vba_30.c| 18 +---
 .../dc/dml/dcn31/display_mode_vba_31.c| 13 +---
 .../dc/dml/dcn314/display_mode_vba_314.c  | 13 +---
 6 files changed, 14 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
index 6e9d7e2b5243..4ca080950924 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
@@ -153,10 +153,10 @@ static unsigned int CalculateVMAndRowBytes(
bool *PTEBufferSizeNotExceeded,
unsigned int *dpte_row_height,
unsigned int *meta_row_height);
-static double CalculateTWait(
+double CalculateTWait(
unsigned int PrefetchMode,
double DRAMClockChangeLatency,
-   double UrgentLatencyPixelDataOnly,
+   double UrgentLatency,
double SREnterPlusExitTime);
 static double CalculateRemoteSurfaceFlipDelay(
struct display_mode_lib *mode_lib,
@@ -2892,20 +2892,20 @@ static void dml20_DisplayPipeConfiguration(struct 
display_mode_lib *mode_lib)
}
 }
 
-static double CalculateTWait(
+double CalculateTWait(
unsigned int PrefetchMode,
double DRAMClockChangeLatency,
-   double UrgentLatencyPixelDataOnly,
+   double UrgentLatency,
double SREnterPlusExitTime)
 {
if (PrefetchMode == 0) {
return dml_max(
-   DRAMClockChangeLatency + 
UrgentLatencyPixelDataOnly,
-   dml_max(SREnterPlusExitTime, 
UrgentLatencyPixelDataOnly));
+   DRAMClockChangeLatency + UrgentLatency,
+   dml_max(SREnterPlusExitTime, UrgentLatency));
} else if (PrefetchMode == 1) {
-   return dml_max(SREnterPlusExitTime, UrgentLatencyPixelDataOnly);
+   return dml_max(SREnterPlusExitTime, UrgentLatency);
} else {
-   return UrgentLatencyPixelDataOnly;
+   return UrgentLatency;
}
 }
 
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
index b02dda8ce70f..2b4dcae4e432 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c
@@ -177,10 +177,10 @@ static unsigned int CalculateVMAndRowBytes(
bool *PTEBufferSizeNotExceeded,
unsigned int *dpte_row_height,
unsigned int *meta_row_height);
-static double CalculateTWait(
+double CalculateTWait(
unsigned int PrefetchMode,
double DRAMClockChangeLatency,
-   double UrgentLatencyPixelDataOnly,
+   double UrgentLatency,
double SREnterPlusExitTime);
 static double CalculateRemoteSurfaceFlipDelay(
struct display_mode_lib *mode_lib,
@@ -2967,23 +2967,6 @@ static void dml20v2_DisplayPipeConfiguration(struct 
display_mode_lib *mode_lib)
}
 }
 
-static double CalculateTWait(
-   unsigned int PrefetchMode,
-   double DRAMClockChangeLatency,
-   double UrgentLatencyPixelDataOnly,
-   double SREnterPlusExitTime)
-{
-   if (PrefetchMode == 0) {
-   return dml_max(
-   DRAMClockChangeLatency + 
UrgentLatencyPixelDataOnly,
-   dml_max(SREnterPlusExitTime, 
UrgentLatencyPixelDataOnly));
-   } else if (PrefetchMode == 1) {
-   return dml_max(SREnterPlusExitTime, UrgentLatencyPixelDataOnly);
-   } else {
-   return UrgentLatencyPixelDataOnly;
-   }
-}
-
 static double CalculateRemoteSurfaceFlipDelay(
struct display_mode_lib *mode_lib,
double VRatio,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
index 6be14f55c78d..a3ef3638d979 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
@@ -205,7 +205,7 @@ static unsigned int CalculateVMAndRowBytes(
unsigned int *DPDE0BytesFrame,
unsigned int *MetaPTEBytesFrame);
 
-static double CalculateTWait(
+double CalculateTWait(
   

Re: [PATCH] drm/amdgpu: use dirty framebuffer helper

2022-09-19 Thread root
Hi, I recently experienced lock-ups that only responded to magic sysreq 
reboots when the amdgpu module was loading on my pc (Athlon II X4 640 CPU,
with Radeon R7 250 - Cape Verde).

.config has:

CONFIG_DRM_AMDGPU=m
CONFIG_DRM_AMDGPU_SI=y
# CONFIG_DRM_AMDGPU_CIK is not set
# CONFIG_DRM_AMDGPU_USERPTR is not set

kernel command line has:

amdgpu.audio=1 amdgpu.si_support=1 radeon.si_support=0 page_owner=on \
amdgpu.gpu_recovery=1

Bisecting lead to:

commit 66f99628eb24409cb8feb5061f78283c8b65f820
Author: Hamza Mahfooz 
Date:   Tue Sep 6 15:01:49 2022 -0400

drm/amdgpu: use dirty framebuffer helper

Currently, we aren't handling DRM_IOCTL_MODE_DIRTYFB. So, use
drm_atomic_helper_dirtyfb() as the dirty callback in the amdgpu_fb_funcs
struct.

Signed-off-by: Hamza Mahfooz 
Acked-by: Alex Deucher 
Signed-off-by: Alex Deucher 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index c20922a5af9f..5b09c8f4fe95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -496,6 +497,7 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector 
*amdgpu_connector,
 static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
.destroy = drm_gem_fb_destroy,
.create_handle = drm_gem_fb_create_handle,
+   .dirty = drm_atomic_helper_dirtyfb,
 };
 
 uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,

After doing a git bisect reset, git pull and reverting the patch above, I
rebuilt the kernel and am successfully running with the amdgpu module loaded
and using the Radeon R7 250 GPU.

I am happy to supply any further configuration details.

Arthur Marsh.


[REGRESSION] Graphical issues on Lenovo Yoga 7 14ARB7 laptop since v6.0-rc1 (bisected)

2022-09-19 Thread August Wikerfors

Hi,
with every kernel version since v6.0-rc1, including the latest git 
master, there are constant graphical issues on this laptop, such as 
heavy stuttering (this is especially noticeable while typing on the 
keyboard), parts of the screen showing random noise, and the entire 
desktop environment freezing.


I bisected the issue which showed that this is the first bad commit:


commit 7cc191ee7621b7145c6cc9c18a4e1929bb5f136e
Author: Leo Li 
Date:   Wed Mar 30 12:45:09 2022 -0400

drm/amd/display: Implement MPO PSR SU

[WHY]

For additional power savings, PSR SU (also referred to as PSR2) can be

enabled on eDP panels with PSR SU support.

PSR2 saves more power compared to PSR1 by allowing more opportunities

for the display hardware to be shut down. In comparison to PSR1, Shut
down can now occur in-between frames, as well as in display regions
where there is no visible update. In otherwords, it allows for some
display hw components to be enabled only for a **selectively updated**
region of the visible display. Hence PSR SU.

[HOW]

To define the SU region, support from the OS is required. OS needs to

inform driver of damaged regions that need to be flushed to the eDP
panel. Today, such support is lacking in most compositors.

Therefore, an in-between solution is to implement PSR SU for MPO and

cursor scenarios. The plane bounds can be used to define the damaged
region to be flushed to panel. This is achieved by:

* Leveraging dm_crtc_state->mpo_requested flag to identify when MPO is

  enabled.
* If MPO is enabled, only add updated plane bounds to dirty region.
  Determine plane update by either:
* Existence of drm damaged clips attached to the plane (added by a
  damage-aware compositor)
* Change in fb id (flip)
* Change in plane bounds (position and dimensions)
* If cursor is enabled, the old_pos and new_pos of cursor plus cursor
  size is used as damaged regions(*).

(*) Cursor updates follow a different code path through DC. PSR SU for

cursor is already implemented in DC, and the only thing required to
enable is to set DC_PSR_VERSION_SU_1 on the eDP link. See
dcn10_dmub_update_cursor_data().

Signed-off-by: Leo Li 

Acked-by: Leo Li 
Reviewed-by: Harry Wentland 
Signed-off-by: Alex Deucher 


#regzbot introduced: 7cc191ee7621b7145c6cc9c18a4e1929bb5f136e

Note that while bisecting I also needed to apply commit 
9946e39fe8d0a5da9eb947d8e40a7ef204ba016e as the keyboard doesn't work 
without it.


Laptop model: Lenovo Yoga 7 14ARB7
CPU: AMD Ryzen 5 6600U
Kernel config: 
https://raw.githubusercontent.com/archlinux/svntogit-packages/aa564cf7088b1d834ef4cda9cb48ff0283fde5c5/trunk/config

Distribution: Arch Linux
Desktop environment: KDE Plasma 5.25.5

lspci:

$ lspci -nn
00:00.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:14b5] (rev 01)
00:00.2 IOMMU [0806]: Advanced Micro Devices, Inc. [AMD] Device [1022:14b6]
00:01.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:14b7] (rev 01)
00:02.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:14b7] (rev 01)
00:02.3 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device [1022:14ba]
00:02.4 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device [1022:14ba]
00:02.5 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device [1022:14ba]
00:03.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:14b7] (rev 01)
00:03.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device [1022:14cd]
00:04.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:14b7] (rev 01)
00:08.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:14b7] (rev 01)
00:08.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:14b9] (rev 10)
00:08.3 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:14b9] (rev 10)
00:14.0 SMBus [0c05]: Advanced Micro Devices, Inc. [AMD] FCH SMBus Controller 
[1022:790b] (rev 71)
00:14.3 ISA bridge [0601]: Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge 
[1022:790e] (rev 51)
00:18.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:1679]
00:18.1 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:167a]
00:18.2 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:167b]
00:18.3 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:167c]
00:18.4 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:167d]
00:18.5 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:167e]
00:18.6 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:167f]
00:18.7 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:1680]
01:00.0 Network controller [0280]: MEDIATEK Corp. MT7922 802.11ax PCI Express 
Wireless 

Re: drm/amdgpu: use dirty framebuffer helper

2022-09-19 Thread Arthur Marsh
I have done a delayed load of amdgpu with the 6.0-rc6 kernel using:

modprobe amdgpu si_support=1

and saved the dmesg output:

[  455.424263] udevd[414]: specified group 'sgx' unknown
[  455.514818] ACPI: bus type drm_connector registered
[  457.759316] [drm] amdgpu kernel modesetting enabled.
[  457.759491] amdgpu :01:00.0: vgaarb: deactivate vga console
[  457.760459] Console: switching to colour dummy device 80x25
[  457.760689] [drm] initializing kernel modesetting (VERDE 0x1002:0x682B 
0x1458:0x22CA 0x87).
[  457.760717] [drm] register mmio base: 0xFE8C
[  457.760720] [drm] register mmio size: 262144
[  457.760872] [drm] add ip block number 0 
[  457.760887] [drm] add ip block number 1 
[  457.760890] [drm] add ip block number 2 
[  457.760893] [drm] add ip block number 3 
[  457.760896] [drm] add ip block number 4 
[  457.760898] [drm] add ip block number 5 
[  457.760901] [drm] add ip block number 6 
[  457.760903] [drm] add ip block number 7 
[  457.804366] [drm] BIOS signature incorrect 20 7
[  457.804376] resource sanity check: requesting [mem 0x000c-0x000d], 
which spans more than PCI Bus :00 [mem 0x000d-0x000d window]
[  457.804383] caller pci_map_rom+0x68/0x1b0 mapping multiple BARs
[  457.804398] amdgpu :01:00.0: No more image in the PCI ROM
[  457.805746] amdgpu :01:00.0: amdgpu: Fetched VBIOS from ROM BAR
[  457.805752] amdgpu: ATOM BIOS: xxx-xxx-xxx
[  457.805775] amdgpu :01:00.0: amdgpu: Trusted Memory Zone (TMZ) feature 
not supported
[  457.805781] amdgpu :01:00.0: amdgpu: PCIE atomic ops is not supported
[  457.806204] [drm] PCIE gen 2 link speeds already enabled
[  457.806632] [drm] vm size is 64 GB, 2 levels, block size is 10-bit, fragment 
size is 9-bit
[  457.869590] amdgpu :01:00.0: amdgpu: VRAM: 2048M 0x00F4 - 
0x00F47FFF (2048M used)
[  457.869605] amdgpu :01:00.0: amdgpu: GART: 1024M 0x00FF - 
0x00FF3FFF
[  457.869622] [drm] Detected VRAM RAM=2048M, BAR=256M
[  457.869625] [drm] RAM width 128bits DDR3
[  457.869706] [drm] amdgpu: 2048M of VRAM memory ready
[  457.869710] [drm] amdgpu: 3979M of GTT memory ready.
[  457.869734] [drm] GART: num cpu pages 262144, num gpu pages 262144
[  457.870061] amdgpu :01:00.0: amdgpu: PCIE GART of 1024M enabled (table 
at 0x00F400A0).
[  457.908024] [drm] Internal thermal controller with fan control
[  457.908045] [drm] amdgpu: dpm initialized
[  457.908126] [drm] AMDGPU Display Connectors
[  457.908128] [drm] Connector 0:
[  457.908131] [drm]   HDMI-A-1
[  457.908133] [drm]   HPD1
[  457.908135] [drm]   DDC: 0x194c 0x194c 0x194d 0x194d 0x194e 0x194e 0x194f 
0x194f
[  457.908139] [drm]   Encoders:
[  457.908141] [drm] DFP1: INTERNAL_UNIPHY
[  457.908144] [drm] Connector 1:
[  457.908145] [drm]   DVI-D-1
[  457.908147] [drm]   HPD2
[  457.908149] [drm]   DDC: 0x1950 0x1950 0x1951 0x1951 0x1952 0x1952 0x1953 
0x1953
[  457.908153] [drm]   Encoders:
[  457.908155] [drm] DFP2: INTERNAL_UNIPHY
[  457.908157] [drm] Connector 2:
[  457.908159] [drm]   VGA-1
[  457.908160] [drm]   DDC: 0x1970 0x1970 0x1971 0x1971 0x1972 0x1972 0x1973 
0x1973
[  457.908164] [drm]   Encoders:
[  457.908166] [drm] CRT1: INTERNAL_KLDSCP_DAC1
[  457.959506] [drm] Found UVD firmware Version: 64.0 Family ID: 13
[  458.497761] [drm] UVD initialized successfully.
[  458.498549] amdgpu :01:00.0: amdgpu: SE 1, SH per SE 2, CU per SH 5, 
active_cu_number 8
[  458.836681] [drm] Initialized amdgpu 3.48.0 20150101 for :01:00.0 on 
minor 0
[  458.909127] fbcon: amdgpudrmfb (fb0) is primary device
[  458.936086] Console: switching to colour frame buffer device 240x67
[  458.936126] BUG: kernel NULL pointer dereference, address: 0010
[  458.936128] #PF: supervisor read access in kernel mode
[  458.936130] #PF: error_code(0x) - not-present page
[  458.936132] PGD 0 P4D 0 
[  458.936134] Oops:  [#1] PREEMPT SMP NOPTI
[  458.936137] CPU: 3 PID: 81 Comm: kworker/3:1 Not tainted 6.0.0-rc6 #5144
[  458.936140] Hardware name: System manufacturer System Product Name/M3A78 
PRO, BIOS 170101/27/2011
[  458.936141] Workqueue: events drm_fb_helper_damage_work [drm_kms_helper]
[  458.936162] RIP: 0010:drm_atomic_helper_dirtyfb+0x13c/0x240 [drm_kms_helper]
[  458.936176] Code: 05 c0 02 00 00 4c 8d 7a f8 48 39 c2 74 58 49 8b 74 24 48 
4d 8d 77 20 4c 89 f7 e8 0f b5 f5 ff 85 c0 75 4b 49 8b 87 58 02 00 00 <48> 39 68 
10 75 bf 4c 89 fe 4c 89 e7 e8 23 bf f5 ff 48 3d 00 f0 ff
[  458.936178] RSP: 0018:b75080db3da8 EFLAGS: 00010246
[  458.936180] RAX:  RBX: 9dbae32e2880 RCX: 003b
[  458.936181] RDX: b75080db3dd8 RSI: b75080db3dd8 RDI: 9dbad6329c48
[  458.936182] RBP: 9dbae9b41100 R08: b75080db3dd8 R09: 9dbae33e01f8
[  458.936184] R10: 9dbadf388028 R11: 0001 R12: 9dbad150e880
[  458.936185] R13: 9dbaee0d9980 R14: 9dbad6329c20 R15: 9dbad6329c00
[  458.936186] FS:  

Re: [PATCH] gpu: dc: fix enum conversion in display_mode_vba

2022-09-19 Thread Wei Yongjun



On 2022/9/19 9:41, Zeng Heng wrote:
> Fix below compile warning when open enum-conversion
> option check (compiled with -Wenum-conversion):
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:
> In function ‘dml20_ModeSupportAndSystemConfigurationFull’:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3900:44:
> error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
> [-Werror=enum-conversion]
>  3900 | locals->ODMCombineEnablePerState[i][k] = false;
>   |^
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3904:46:
> error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
> [-Werror=enum-conversion]
>  3904 |   locals->ODMCombineEnablePerState[i][k] = true;
>   |  ^
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3907:46:
> error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
> [-Werror=enum-conversion]
>  3907 |   locals->ODMCombineEnablePerState[i][k] = true;
>   |  ^
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3960:45:
> error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
> [-Werror=enum-conversion]
>  3960 |  locals->ODMCombineEnablePerState[i][k] = false;
> 
> Use the proper value from the right enumerated type,
> dm_odm_combine_mode_disabled & dm_odm_combine_mode_2to1,
> so there is no more implicit conversion.
> 
> The numerical values of dm_odm_combine_mode_disabled
> & false and dm_odm_combine_mode_2to1 & true
> happen to be the same, so there is no change in
> behavior.
> 

LGTM



[PATCH] drm/amd/display: refactor CalculateWriteBackDelay to use vba_vars_st ptr

2022-09-19 Thread Tom Rix
Mimimize the function signature by passing a pointer and an index instead
of passing several elements of the pointer.

The dml2x,dml3x families uses the same algorithm.  Remove the duplicates.
Use dml20_ and dml30_ prefix to distinguish the two variants.

Signed-off-by: Tom Rix 
---
 .../dc/dml/dcn20/display_mode_vba_20.c|  78 +++-
 .../dc/dml/dcn20/display_mode_vba_20v2.c  | 115 ++
 .../dc/dml/dcn21/display_mode_vba_21.c| 114 +
 .../dc/dml/dcn30/display_mode_vba_30.c|  74 +++
 .../dc/dml/dcn31/display_mode_vba_31.c|  76 +---
 .../dc/dml/dcn314/display_mode_vba_314.c  |  76 +---
 .../dc/dml/dcn32/display_mode_vba_32.c|  42 +--
 .../dc/dml/dcn32/display_mode_vba_util_32.c   |  30 -
 .../dc/dml/dcn32/display_mode_vba_util_32.h   |  10 +-
 9 files changed, 63 insertions(+), 552 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
index d3b5b6fedf04..6e9d7e2b5243 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
@@ -217,16 +217,8 @@ static void CalculateFlipSchedule(
double *DestinationLinesToRequestRowInImmediateFlip,
double *final_flip_bw,
bool *ImmediateFlipSupportedForPipe);
-static double CalculateWriteBackDelay(
-   enum source_format_class WritebackPixelFormat,
-   double WritebackHRatio,
-   double WritebackVRatio,
-   unsigned int WritebackLumaHTaps,
-   unsigned int WritebackLumaVTaps,
-   unsigned int WritebackChromaHTaps,
-   unsigned int WritebackChromaVTaps,
-   unsigned int WritebackDestinationWidth);
 
+double dlm20_CalculateWriteBackDelay(struct vba_vars_st *vba, unsigned int i);
 static void dml20_DisplayPipeConfiguration(struct display_mode_lib *mode_lib);
 static void 
dml20_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
struct display_mode_lib *mode_lib);
@@ -1085,6 +1077,7 @@ static unsigned int CalculateVMAndRowBytes(
 static void 
dml20_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
struct display_mode_lib *mode_lib)
 {
+   struct vba_vars_st *v = _lib->vba;
unsigned int j, k;
 
mode_lib->vba.WritebackDISPCLK = 0.0;
@@ -1980,36 +1973,15 @@ static void 
dml20_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPer
if (mode_lib->vba.BlendingAndTiming[k] == k) {
if (mode_lib->vba.WritebackEnable[k] == true) {

mode_lib->vba.WritebackDelay[mode_lib->vba.VoltageLevel][k] =
-   mode_lib->vba.WritebackLatency
-   + 
CalculateWriteBackDelay(
-   
mode_lib->vba.WritebackPixelFormat[k],
-   
mode_lib->vba.WritebackHRatio[k],
-   
mode_lib->vba.WritebackVRatio[k],
-   
mode_lib->vba.WritebackLumaHTaps[k],
-   
mode_lib->vba.WritebackLumaVTaps[k],
-   
mode_lib->vba.WritebackChromaHTaps[k],
-   
mode_lib->vba.WritebackChromaVTaps[k],
-   
mode_lib->vba.WritebackDestinationWidth[k])
-   
/ mode_lib->vba.DISPCLK;
+   mode_lib->vba.WritebackLatency + 
dlm20_CalculateWriteBackDelay(v, k) / mode_lib->vba.DISPCLK;
} else

mode_lib->vba.WritebackDelay[mode_lib->vba.VoltageLevel][k] = 0;
for (j = 0; j < mode_lib->vba.NumberOfActivePlanes; 
++j) {
if (mode_lib->vba.BlendingAndTiming[j] == k
&& 
mode_lib->vba.WritebackEnable[j] == true) {

mode_lib->vba.WritebackDelay[mode_lib->vba.VoltageLevel][k] =
-   dml_max(
-   
mode_lib->vba.WritebackDelay[mode_lib->vba.VoltageLevel][k],
-

[PATCH] gpu: dc: fix enum conversion in display_mode_vba

2022-09-19 Thread Zeng Heng
Fix below compile warning when open enum-conversion
option check (compiled with -Wenum-conversion):

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:
In function ‘dml20_ModeSupportAndSystemConfigurationFull’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3900:44:
error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
[-Werror=enum-conversion]
 3900 | locals->ODMCombineEnablePerState[i][k] = false;
  |^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3904:46:
error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
[-Werror=enum-conversion]
 3904 |   locals->ODMCombineEnablePerState[i][k] = true;
  |  ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3907:46:
error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
[-Werror=enum-conversion]
 3907 |   locals->ODMCombineEnablePerState[i][k] = true;
  |  ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3960:45:
error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
[-Werror=enum-conversion]
 3960 |  locals->ODMCombineEnablePerState[i][k] = false;

Use the proper value from the right enumerated type,
dm_odm_combine_mode_disabled & dm_odm_combine_mode_2to1,
so there is no more implicit conversion.

The numerical values of dm_odm_combine_mode_disabled
& false and dm_odm_combine_mode_2to1 & true
happen to be the same, so there is no change in
behavior.

Signed-off-by: Zeng Heng 
---
 .../amd/display/dc/dml/dcn20/display_mode_vba_20.c   |  8 
 .../amd/display/dc/dml/dcn20/display_mode_vba_20v2.c | 10 +-
 .../amd/display/dc/dml/dcn21/display_mode_vba_21.c   | 12 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
index d3b5b6fedf04..6266b0788387 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
@@ -3897,14 +3897,14 @@ void dml20_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l

mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine = mode_lib->vba.PixelClock[k] 
/ 2
* (1 + 
mode_lib->vba.DISPCLKDPPCLKDSCCLKDownSpreading / 100.0);
 
-   locals->ODMCombineEnablePerState[i][k] = false;
+   locals->ODMCombineEnablePerState[i][k] = 
dm_odm_combine_mode_disabled;
mode_lib->vba.PlaneRequiredDISPCLK = 
mode_lib->vba.PlaneRequiredDISPCLKWithoutODMCombine;
if (mode_lib->vba.ODMCapability) {
if 
(locals->PlaneRequiredDISPCLKWithoutODMCombine > 
mode_lib->vba.MaxDispclkRoundedDownToDFSGranularity) {
-   
locals->ODMCombineEnablePerState[i][k] = true;
+   
locals->ODMCombineEnablePerState[i][k] = dm_odm_combine_mode_2to1;

mode_lib->vba.PlaneRequiredDISPCLK = 
mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine;
} else if (locals->HActive[k] > 
DCN20_MAX_420_IMAGE_WIDTH && locals->OutputFormat[k] == dm_420) {
-   
locals->ODMCombineEnablePerState[i][k] = true;
+   
locals->ODMCombineEnablePerState[i][k] = dm_odm_combine_mode_2to1;

mode_lib->vba.PlaneRequiredDISPCLK = 
mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine;
}
}
@@ -3957,7 +3957,7 @@ void dml20_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
locals->RequiredDISPCLK[i][j] = 0.0;
locals->DISPCLK_DPPCLK_Support[i][j] = true;
for (k = 0; k <= 
mode_lib->vba.NumberOfActivePlanes - 1; k++) {
-   locals->ODMCombineEnablePerState[i][k] 
= false;
+   locals->ODMCombineEnablePerState[i][k] 
= dm_odm_combine_mode_disabled;
if (locals->SwathWidthYSingleDPP[k] <= 
locals->MaximumSwathWidth[k]) {
locals->NoOfDPP[i][j][k] = 1;
locals->RequiredDPPCLK[i][j][k] 
= locals->MinDPPCLKUsingSingleDPP[k]
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20v2.c 

mainline build failure (new) for x86_64 allmodconfig with clang

2022-09-19 Thread Sudip Mukherjee (Codethink)
Hi All,

The latest mainline kernel branch fails to build for x86_64 allmodconfig
with clang. The errors are:

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn314/display_mode_vba_314.c:4020:6:
 error: stack frame size (2184) exceeds limit (2048) in 
'dml314_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
void dml314_ModeSupportAndSystemConfigurationFull(struct display_mode_lib 
*mode_lib)
 ^
1 error generated.


Note: This is a new error seen on top on a335366bad13 ("Merge tag 
'gpio-fixes-for-v6.0-rc6' of 
git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux").
Previous reported clang build error is now fixed, thanks to Nathan.

And, it appears Nathan has already sent a fix for this:
https://github.com/intel-lab-lkp/linux/commit/4ecc45d7585ae2e05d622879ad97e13a7d8c595b
https://github.com/intel-lab-lkp/linux/commit/819976a950b497d7f10cd9a198a94c26a9005b30


--
Regards
Sudip



Re: [PATCH] gpu: dc: fix enum conversion in display_mode_vba

2022-09-19 Thread Christian König

Am 19.09.22 um 03:41 schrieb Zeng Heng:

Fix below compile warning when open enum-conversion
option check (compiled with -Wenum-conversion):

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:
In function ‘dml20_ModeSupportAndSystemConfigurationFull’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3900:44:
error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
[-Werror=enum-conversion]
  3900 | locals->ODMCombineEnablePerState[i][k] = false;
   |^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3904:46:
error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
[-Werror=enum-conversion]
  3904 |   locals->ODMCombineEnablePerState[i][k] = true;
   |  ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3907:46:
error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
[-Werror=enum-conversion]
  3907 |   locals->ODMCombineEnablePerState[i][k] = true;
   |  ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3960:45:
error: implicit conversion from ‘enum ’ to ‘enum odm_combine_mode’ 
[-Werror=enum-conversion]
  3960 |  locals->ODMCombineEnablePerState[i][k] = false;

Use the proper value from the right enumerated type,
dm_odm_combine_mode_disabled & dm_odm_combine_mode_2to1,
so there is no more implicit conversion.

The numerical values of dm_odm_combine_mode_disabled
& false and dm_odm_combine_mode_2to1 & true
happen to be the same, so there is no change in
behavior.


In the subject line the correct prefix is "drm/amdgpu: ", but apart 
from that looks good to me as well.


But our DC team has to take a closer look.

Thanks,
Christian.



Signed-off-by: Zeng Heng 
---
  .../amd/display/dc/dml/dcn20/display_mode_vba_20.c   |  8 
  .../amd/display/dc/dml/dcn20/display_mode_vba_20v2.c | 10 +-
  .../amd/display/dc/dml/dcn21/display_mode_vba_21.c   | 12 ++--
  3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
index d3b5b6fedf04..6266b0788387 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c
@@ -3897,14 +3897,14 @@ void dml20_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l

mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine = mode_lib->vba.PixelClock[k] 
/ 2
* (1 + 
mode_lib->vba.DISPCLKDPPCLKDSCCLKDownSpreading / 100.0);
  
-locals->ODMCombineEnablePerState[i][k] = false;

+   locals->ODMCombineEnablePerState[i][k] = 
dm_odm_combine_mode_disabled;
mode_lib->vba.PlaneRequiredDISPCLK = 
mode_lib->vba.PlaneRequiredDISPCLKWithoutODMCombine;
if (mode_lib->vba.ODMCapability) {
if 
(locals->PlaneRequiredDISPCLKWithoutODMCombine > 
mode_lib->vba.MaxDispclkRoundedDownToDFSGranularity) {
-   
locals->ODMCombineEnablePerState[i][k] = true;
+   
locals->ODMCombineEnablePerState[i][k] = dm_odm_combine_mode_2to1;
mode_lib->vba.PlaneRequiredDISPCLK 
= mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine;
} else if (locals->HActive[k] > 
DCN20_MAX_420_IMAGE_WIDTH && locals->OutputFormat[k] == dm_420) {
-   
locals->ODMCombineEnablePerState[i][k] = true;
+   
locals->ODMCombineEnablePerState[i][k] = dm_odm_combine_mode_2to1;
mode_lib->vba.PlaneRequiredDISPCLK 
= mode_lib->vba.PlaneRequiredDISPCLKWithODMCombine;
}
}
@@ -3957,7 +3957,7 @@ void dml20_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
locals->RequiredDISPCLK[i][j] = 0.0;
locals->DISPCLK_DPPCLK_Support[i][j] = true;
for (k = 0; k <= 
mode_lib->vba.NumberOfActivePlanes - 1; k++) {
-   locals->ODMCombineEnablePerState[i][k] 
= false;
+   locals->ODMCombineEnablePerState[i][k] 
= dm_odm_combine_mode_disabled;
if (locals->SwathWidthYSingleDPP[k] <= 
locals->MaximumSwathWidth[k]) {
locals->NoOfDPP[i][j][k] = 1;
 

Re: [PATCH] drm/amdgpu: use dirty framebuffer helper

2022-09-19 Thread Thomas Zimmermann

Hi

Am 06.09.22 um 21:57 schrieb Hamza Mahfooz:

Currently, we aren't handling DRM_IOCTL_MODE_DIRTYFB. So, use
drm_atomic_helper_dirtyfb() as the dirty callback in the amdgpu_fb_funcs
struct.


drm_atomic_helper_dirtyfb() creates a new atomic commit for the 
frambuffer's planes. Drivers can then updates these planes' output 
(e.g., writeback to video memory). I thought that amdgpu simply scans 
out from the framebuffer's memory regions in VRAM. So I'm curious why 
this patch is necessary.


Best regards
Thomas



Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index c20922a5af9f..5b09c8f4fe95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -38,6 +38,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -496,6 +497,7 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector 
*amdgpu_connector,
  static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
.destroy = drm_gem_fb_destroy,
.create_handle = drm_gem_fb_create_handle,
+   .dirty = drm_atomic_helper_dirtyfb,
  };
  
  uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature