[PATCH] drm: amd: display: fix kernel-doc struct warning
Fix a kernel-doc warning of missing struct field desription:
../drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:331: warning: Function
parameter or member 'hdcp_workqueue' not described in 'amdgpu_display_manager'
Fixes: 52704fcaf74b ("drm/amd/display: Initialize HDCP work queue")
Signed-off-by: Randy Dunlap
Cc: Bhawanpreet Lakha
Cc: Harry Wentland
Cc: Alex Deucher
Cc: Leo Li
Cc: amd-gfx@lists.freedesktop.org
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |1 +
1 file changed, 1 insertion(+)
--- lnx-57-rc2.orig/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ lnx-57-rc2/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -139,6 +139,7 @@ struct amdgpu_dm_backlight_caps {
* @backlight_link: Link on which to control backlight
* @backlight_caps: Capabilities of the backlight device
* @freesync_module: Module handling freesync calculations
+ * @hdcp_workqueue: workqueue for display manager interaction with HDCP module
* @fw_dmcu: Reference to DMCU firmware
* @dmcu_fw_version: Version of the DMCU firmware
* @soc_bounding_box: SOC bounding box values provided by gpu_info FW
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm: amdgpu: fix kernel-doc struct warning
Fix a kernel-doc warning of missing struct field desription:
../drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:92: warning: Function parameter or
member 'vm' not described in 'amdgpu_vm_eviction_lock'
Fixes: a269e44989f3 ("drm/amdgpu: Avoid reclaim fs while eviction lock")
Signed-off-by: Randy Dunlap
Cc: Signed-off-by: Alex Sierra
Cc: Felix Kuehling
Cc: Christian König
Cc: Alex Deucher
Cc: David (ChunMing) Zhou
Cc: amd-gfx@lists.freedesktop.org
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- lnx-57-rc2.orig/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ lnx-57-rc2/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -82,7 +82,7 @@ struct amdgpu_prt_cb {
struct dma_fence_cb cb;
};
-/**
+/*
* vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
* happens while holding this lock anywhere to prevent deadlocks when
* an MMU notifier runs in reclaim-FS context.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: remove unneeded conversion to bool
The '==' expression itself is bool, no need to convert it to bool again. This fixes the following coccicheck warning: drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dpp_cm.c:485:66-71: WARNING: conversion to bool not needed here drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dpp_cm.c:896:68-73: WARNING: conversion to bool not needed here Signed-off-by: Jason Yan --- drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dpp_cm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dpp_cm.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dpp_cm.c index 8dc3d1f73984..2feb051a2002 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dpp_cm.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dpp_cm.c @@ -482,7 +482,7 @@ bool dpp20_program_blnd_lut( next_mode = LUT_RAM_A; dpp20_power_on_blnd_lut(dpp_base, true); - dpp20_configure_blnd_lut(dpp_base, next_mode == LUT_RAM_A ? true:false); + dpp20_configure_blnd_lut(dpp_base, next_mode == LUT_RAM_A); if (next_mode == LUT_RAM_A) dpp20_program_blnd_luta_settings(dpp_base, params); @@ -893,7 +893,7 @@ bool dpp20_program_shaper( else next_mode = LUT_RAM_A; - dpp20_configure_shaper_lut(dpp_base, next_mode == LUT_RAM_A ? true:false); + dpp20_configure_shaper_lut(dpp_base, next_mode == LUT_RAM_A); if (next_mode == LUT_RAM_A) dpp20_program_shaper_luta_settings(dpp_base, params); -- 2.21.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re:Re: [PATCH] drm/amdgpu: Return more error codes in amdgpu_connector_set_property()
发件人：Markus Elfring 发送日期：2020-04-19 17:34:47 收件人：Bernard Zhao ,amd-gfx@lists.freedesktop.org,dri-de...@lists.freedesktop.org,Alex Deucher ,"Christian König" ,Chunming Zhou 抄送人：Andrzej Pietrasiewicz ,Daniel Vetter ,David Airlie ,Dhinakaran Pandiyan ,"José Roberto de Souza" ,Lyude Paul ,Neil Armstrong ,Sam Ravnborg ,linux-ker...@vger.kernel.org,opensource.ker...@vivo.com 主题：Re: [PATCH] drm/amdgpu: Return more error codes in amdgpu_connector_set_property()>> The "if(!encoder)" branch return the same value 0 of the success >> branch, maybe return -EINVAL is more better. > >I suggest to improve the commit message. Sure. > >* Would you like to adjust the patch subject? > >* How do you think about to add the tag “Fixes” > because of adjustments for the exception handling? This is a good idea, the code will be more in line with the Linux specifications. I will adjust this patch commit message and modify the code. I will submit it again. > >Regards, >Markus ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Reg. Adaptive Sync feature in xf86-video-amdgpu
Hello Team, I'm working on adding Adaptive Sync feature in Xserver/modesetting. When understanding the existing AMD's implementation, I've few doubts regarding the vrr property being set on the Window from MESA. I have made the modifications in xserver/modesetting but when i launch the application(DOTA2), below condition gets failed https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/-/blob/master/src/amdgpu_kms.c#L110 As per the code, i had a confusion that this condition will never met. *I had analysed the code and here is the analysis made when the flip happens in xserver When full screen gaming application is opened, variable_refresh property is being set in MESA and xserver will get notified with amdgpu_change_property() method(Same method we registered with the client during AMDGPUScreenInit_KMS()). * *Below actions will happen in ms_change_property() * *1. Create Local WindowPtr and copy the data from Stuff->window to this WindowPtr* *2. Call amdgpu_vrr_property_update() based on the property set in Stuff by passing the WindowPtr to it.* *a. Read Private Keys for WindowPtr in amdgpu_vrr_property_update(). * *b. Compare info->flip_window and this WindowPtr and make a call to amdgpu_present_set_screen_vrr(). → But this method will never gets called due to the condition mismatch every time. Why ? * *Why ?* *info->flip_window gets updated with window (WindowPtr) in amdgpu_present_check_flip() when amdgpu_present_flip() method gets called from DIX. This pointer will never same as the WindowPtr created in amdgpu_change_property() and variable_refresh flag is being set for in amdgpu_change_property() WindowPtr only. * Can you please help me in understanding on this ? JFYI, I have used DOTA2 application to verify this feature. Thanka Uday Kiran ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re:Re: [PATCH] drm/amdgpu: Reduce a lock scope in amdgpu_amdkfd_gpuvm_free_memory_of_gpu()
From: Markus Elfring Date: 2020-04-19 17:05:07 To: Bernard Zhao ,amd-gfx@lists.freedesktop.org,dri-de...@lists.freedesktop.org,Alex Deucher ,"Christian König" ,Chunming Zhou ,"Felix Kühling" Cc: ker...@vivo.com,linux-ker...@vger.kernel.org,Daniel Vetter ,David Airlie Subject: Re: [PATCH] drm/amdgpu: Reduce a lock scope in amdgpu_amdkfd_gpuvm_free_memory_of_gpu()>> Maybe we could reduce the mutex_lock(&mem->lock)`s protected code area, >> and noneed to protect pr_debug. > >I suggest to improve the commit message. >Would you like to adjust the patch subject? > >Do you imagine that data synchronisation should evolve in other ways? > >Regards, >Markus Sure, I will modify the code and adjust this patch subject. I will submit it again. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: refine kiq read register
Christian
>>> Well I was under the assumption that this is actually what is done here.
If that is not the case the patch is a rather clear NAK.
<<<
There are two kinds of problems in the current KIQ reading reg, Yintian tend to
fix one of them but not all ...
The first problem is :
During the sleep of the first KIQ reading, another KIQ reading initiated an the
read back register value flushed the first readback value, thus the first
reading will get the wrong result.
This is the issue yintian's patch to address, by put the readback value not in
a shared WB but in a chunk DW of command submit
The second problem is:
Since we don't utilize GPU scheduler for KIQ submit thus if the KIQ is busy
with some commands then those unfinished commands maybe overwritten by a new
command submit, and that's not the
Problem yintian's patch tend to address. Felex pointed it out which is fine and
we can use another patch to address it, I'm also planning and scoping it.
The optional way is:
1) We use GPU scheduler to manage KIQ activity, and all jobs are submitted to
KIQ through a IB, thus no overwritten will happen
2) we still skip gpu scheduler but always use IB to put jobs on KIQ, thus each
JOB will occupy the fixed space/DW of RB, so we can avoid overwrite unfinished
command
We can discuss the second problem later
Can we first get the first problem done ? thanks
_
Monk Liu|GPU Virtualization Team |AMD
-Original Message-
From: Christian König
Sent: Monday, April 20, 2020 1:03 AM
To: Kuehling, Felix ; Tao, Yintian
; Liu, Monk
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: refine kiq read register
Am 17.04.20 um 17:39 schrieb Felix Kuehling:
> Am 2020-04-17 um 2:53 a.m. schrieb Yintian Tao:
>> According to the current kiq read register method, there will be race
>> condition when using KIQ to read register if multiple clients want to
>> read at same time just like the expample below:
>> 1. client-A start to read REG-0 throguh KIQ 2. client-A poll the
>> seqno-0 3. client-B start to read REG-1 through KIQ 4. client-B poll
>> the seqno-1 5. the kiq complete these two read operation 6. client-A
>> to read the register at the wb buffer and
>> get REG-1 value
>>
>> Therefore, directly make kiq write the register value at the ring
>> buffer then there will be no race condition for the wb buffer.
>>
>> v2: supply the read_clock and move the reg_val_offs back
>>
>> Signed-off-by: Yintian Tao
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 11 --
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 1 -
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 5 +++--
>> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 14 +---
>> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 14 +---
>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 28
>> 6 files changed, 33 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index ea576b4260a4..4e1c0239e561 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -304,10 +304,6 @@ int amdgpu_gfx_kiq_init_ring(struct
>> amdgpu_device *adev,
>>
>> spin_lock_init(&kiq->ring_lock);
>>
>> -r = amdgpu_device_wb_get(adev, &kiq->reg_val_offs);
>> -if (r)
>> -return r;
>> -
>> ring->adev = NULL;
>> ring->ring_obj = NULL;
>> ring->use_doorbell = true;
>> @@ -331,7 +327,6 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device
>> *adev,
>>
>> void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
>> {
>> -amdgpu_device_wb_free(ring->adev, ring->adev->gfx.kiq.reg_val_offs);
>> amdgpu_ring_fini(ring);
>> }
>>
>> @@ -675,12 +670,14 @@ uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev,
>> uint32_t reg)
>> uint32_t seq;
>> struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>> struct amdgpu_ring *ring = &kiq->ring;
>> +uint64_t reg_val_offs = 0;
>>
>> BUG_ON(!ring->funcs->emit_rreg);
>>
>> spin_lock_irqsave(&kiq->ring_lock, flags);
>> amdgpu_ring_alloc(ring, 32);
>> -amdgpu_ring_emit_rreg(ring, reg);
>> +reg_val_offs = (ring->wptr & ring->buf_mask) + 30;
> I think that should be (ring->wptr + 30) & ring->buf_mask. Otherwise
> the reg_val_offset can be past the end of the ring.
>
> But that still leaves a problem if another command is submitted to the
> KIQ before you read the returned reg_val from the ring. Your reg_val
> can be overwritten by the new command and you get the wrong result. Or
> the command can be overwritten with the reg_val, which will most
> likely hang the CP.
>
> You could allocate space on the KIQ ring with a NOP command to prevent
> that space from being overwritten by other commands.
Well I was under the assumption that this is actually what is done here.
If that is not the case the patch is a rather clear
RE: [PATCH] drm/amd/powerplay: update smu12_driver_if.h to align with pmfw
[AMD Official Use Only - Internal Distribution Only]
Acked-by: Huang Rui
-Original Message-
From: Liang, Prike
Sent: Monday, April 20, 2020 10:11 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Huang, Ray
; Liang, Prike
Subject: [PATCH] drm/amd/powerplay: update smu12_driver_if.h to align with pmfw
Update the smu12_driver_if.h header to follow the pmfw release.
Signed-off-by: Prike Liang
Reviewed-by: Alex Deucher
---
.../gpu/drm/amd/powerplay/inc/smu12_driver_if.h| 40 ++
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu12_driver_if.h
b/drivers/gpu/drm/amd/powerplay/inc/smu12_driver_if.h
index 2f85a34..e9315eb 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu12_driver_if.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu12_driver_if.h
@@ -27,7 +27,7 @@
// *** IMPORTANT ***
// SMU TEAM: Always increment the interface version if // any structure is
changed in this file -#define SMU12_DRIVER_IF_VERSION 11
+#define SMU12_DRIVER_IF_VERSION 14
typedef struct {
int32_t value;
@@ -154,15 +154,19 @@ typedef enum {
} CLOCK_IDs_e;
// Throttler Status Bitmask
-#define THROTTLER_STATUS_BIT_SPL0
-#define THROTTLER_STATUS_BIT_FPPT 1
-#define THROTTLER_STATUS_BIT_SPPT 2
-#define THROTTLER_STATUS_BIT_SPPT_APU 3
-#define THROTTLER_STATUS_BIT_THM_CORE 4
-#define THROTTLER_STATUS_BIT_THM_GFX5
-#define THROTTLER_STATUS_BIT_THM_SOC6
-#define THROTTLER_STATUS_BIT_TDC_VDD7
-#define THROTTLER_STATUS_BIT_TDC_SOC8
+#define THROTTLER_STATUS_BIT_SPL0
+#define THROTTLER_STATUS_BIT_FPPT 1
+#define THROTTLER_STATUS_BIT_SPPT 2
+#define THROTTLER_STATUS_BIT_SPPT_APU 3
+#define THROTTLER_STATUS_BIT_THM_CORE 4
+#define THROTTLER_STATUS_BIT_THM_GFX5
+#define THROTTLER_STATUS_BIT_THM_SOC6
+#define THROTTLER_STATUS_BIT_TDC_VDD7
+#define THROTTLER_STATUS_BIT_TDC_SOC8
+#define THROTTLER_STATUS_BIT_PROCHOT_CPU9
+#define THROTTLER_STATUS_BIT_PROCHOT_GFX 10
+#define THROTTLER_STATUS_BIT_EDC_CPU 11
+#define THROTTLER_STATUS_BIT_EDC_GFX 12
typedef struct {
uint16_t ClockFrequency[CLOCK_COUNT]; //[MHz] @@ -180,7 +184,7 @@ typedef
struct {
uint16_t Power[2];//[mW] indices: VDDCR_VDD, VDDCR_SOC
uint16_t FanPwm; //[milli]
- uint16_t CurrentSocketPower; //[mW]
+ uint16_t CurrentSocketPower; //[W]
uint16_t CoreFrequency[8];//[MHz]
uint16_t CorePower[8];//[mW]
@@ -193,10 +197,16 @@ typedef struct {
uint16_t ThrottlerStatus;
uint16_t spare;
- uint16_t StapmOriginalLimit; //[mW]
- uint16_t StapmCurrentLimit; //[mW]
- uint16_t ApuPower; //[mW]
- uint16_t dGpuPower; //[mW]
+ uint16_t StapmOriginalLimit; //[W]
+ uint16_t StapmCurrentLimit; //[W]
+ uint16_t ApuPower;//[W]
+ uint16_t dGpuPower; //[W]
+
+ uint16_t VddTdcValue; //[mA]
+ uint16_t SocTdcValue; //[mA]
+ uint16_t VddEdcValue; //[mA]
+ uint16_t SocEdcValue; //[mA]
+ uint16_t reserve[2];
} SmuMetrics_t;
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Fix kernel panic while gpu recovery
Ras error occurs while gpu recovery. We can not add its list head
to two lists at same time.
Signed-off-by: xinhui pan
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 68b82f7b0b80..d753c38c3b1d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1449,20 +1449,16 @@ static void amdgpu_ras_do_recovery(struct work_struct
*work)
container_of(work, struct amdgpu_ras, recovery_work);
struct amdgpu_device *remote_adev = NULL;
struct amdgpu_device *adev = ras->adev;
- struct list_head device_list, *device_list_handle = NULL;
struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, false);
/* Build list of devices to query RAS related errors */
- if (hive && adev->gmc.xgmi.num_physical_nodes > 1)
- device_list_handle = &hive->device_list;
- else {
- INIT_LIST_HEAD(&device_list);
- list_add_tail(&adev->gmc.xgmi.head, &device_list);
- device_list_handle = &device_list;
- }
-
- list_for_each_entry(remote_adev, device_list_handle, gmc.xgmi.head) {
- amdgpu_ras_log_on_err_counter(remote_adev);
+ if (hive && adev->gmc.xgmi.num_physical_nodes > 1) {
+ list_for_each_entry(remote_adev, &hive->device_list,
+ gmc.xgmi.head) {
+ amdgpu_ras_log_on_err_counter(remote_adev);
+ }
+ } else {
+ amdgpu_ras_log_on_err_counter(adev);
}
if (amdgpu_device_should_recover_gpu(ras->adev))
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: refine kiq read register
Hi Felix
Many thanks for your review. I have modified it according to your comments and
suggestion.
Best Regards
Yintian Tao
-Original Message-
From: Kuehling, Felix
Sent: 2020年4月17日 23:39
To: Tao, Yintian ; Liu, Monk
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: refine kiq read register
Am 2020-04-17 um 2:53 a.m. schrieb Yintian Tao:
> According to the current kiq read register method, there will be race
> condition when using KIQ to read register if multiple clients want to
> read at same time just like the expample below:
> 1. client-A start to read REG-0 throguh KIQ 2. client-A poll the
> seqno-0 3. client-B start to read REG-1 through KIQ 4. client-B poll
> the seqno-1 5. the kiq complete these two read operation 6. client-A
> to read the register at the wb buffer and
>get REG-1 value
>
> Therefore, directly make kiq write the register value at the ring
> buffer then there will be no race condition for the wb buffer.
>
> v2: supply the read_clock and move the reg_val_offs back
>
> Signed-off-by: Yintian Tao
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 11 --
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 1 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 5 +++--
> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 14 +---
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 14 +---
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 28
> 6 files changed, 33 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index ea576b4260a4..4e1c0239e561 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -304,10 +304,6 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device
> *adev,
>
> spin_lock_init(&kiq->ring_lock);
>
> - r = amdgpu_device_wb_get(adev, &kiq->reg_val_offs);
> - if (r)
> - return r;
> -
> ring->adev = NULL;
> ring->ring_obj = NULL;
> ring->use_doorbell = true;
> @@ -331,7 +327,6 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device
> *adev,
>
> void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring) {
> - amdgpu_device_wb_free(ring->adev, ring->adev->gfx.kiq.reg_val_offs);
> amdgpu_ring_fini(ring);
> }
>
> @@ -675,12 +670,14 @@ uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev,
> uint32_t reg)
> uint32_t seq;
> struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> struct amdgpu_ring *ring = &kiq->ring;
> + uint64_t reg_val_offs = 0;
>
> BUG_ON(!ring->funcs->emit_rreg);
>
> spin_lock_irqsave(&kiq->ring_lock, flags);
> amdgpu_ring_alloc(ring, 32);
> - amdgpu_ring_emit_rreg(ring, reg);
> + reg_val_offs = (ring->wptr & ring->buf_mask) + 30;
I think that should be (ring->wptr + 30) & ring->buf_mask. Otherwise the
reg_val_offset can be past the end of the ring.
But that still leaves a problem if another command is submitted to the KIQ
before you read the returned reg_val from the ring. Your reg_val can be
overwritten by the new command and you get the wrong result. Or the command can
be overwritten with the reg_val, which will most likely hang the CP.
You could allocate space on the KIQ ring with a NOP command to prevent that
space from being overwritten by other commands.
Regards,
Felix
> + amdgpu_ring_emit_rreg(ring, reg, reg_val_offs);
> amdgpu_fence_emit_polling(ring, &seq);
> amdgpu_ring_commit(ring);
> spin_unlock_irqrestore(&kiq->ring_lock, flags); @@ -707,7 +704,7 @@
> uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> if (cnt > MAX_KIQ_REG_TRY)
> goto failed_kiq_read;
>
> - return adev->wb.wb[kiq->reg_val_offs];
> + return ring->ring[reg_val_offs];
>
> failed_kiq_read:
> pr_err("failed to read reg:%x\n", reg); diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 634746829024..ee698f0246d8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -103,7 +103,6 @@ struct amdgpu_kiq {
> struct amdgpu_ring ring;
> struct amdgpu_irq_src irq;
> const struct kiq_pm4_funcs *pmf;
> - uint32_treg_val_offs;
> };
>
> /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index f61664ee4940..a3d88f2aa9f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -181,7 +181,8 @@ struct amdgpu_ring_funcs {
> void (*end_use)(struct amdgpu_ring *ring);
> void (*emit_switch_buffer) (struct amdgpu_ring *ring);
> void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
> - void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
> + void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg,
> +
[PATCH] drm/amd/powerplay: update smu12_driver_if.h to align with pmfw
Update the smu12_driver_if.h header to follow the pmfw release.
Signed-off-by: Prike Liang
Reviewed-by: Alex Deucher
---
.../gpu/drm/amd/powerplay/inc/smu12_driver_if.h| 40 ++
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu12_driver_if.h
b/drivers/gpu/drm/amd/powerplay/inc/smu12_driver_if.h
index 2f85a34..e9315eb 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu12_driver_if.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu12_driver_if.h
@@ -27,7 +27,7 @@
// *** IMPORTANT ***
// SMU TEAM: Always increment the interface version if
// any structure is changed in this file
-#define SMU12_DRIVER_IF_VERSION 11
+#define SMU12_DRIVER_IF_VERSION 14
typedef struct {
int32_t value;
@@ -154,15 +154,19 @@ typedef enum {
} CLOCK_IDs_e;
// Throttler Status Bitmask
-#define THROTTLER_STATUS_BIT_SPL0
-#define THROTTLER_STATUS_BIT_FPPT 1
-#define THROTTLER_STATUS_BIT_SPPT 2
-#define THROTTLER_STATUS_BIT_SPPT_APU 3
-#define THROTTLER_STATUS_BIT_THM_CORE 4
-#define THROTTLER_STATUS_BIT_THM_GFX5
-#define THROTTLER_STATUS_BIT_THM_SOC6
-#define THROTTLER_STATUS_BIT_TDC_VDD7
-#define THROTTLER_STATUS_BIT_TDC_SOC8
+#define THROTTLER_STATUS_BIT_SPL0
+#define THROTTLER_STATUS_BIT_FPPT 1
+#define THROTTLER_STATUS_BIT_SPPT 2
+#define THROTTLER_STATUS_BIT_SPPT_APU 3
+#define THROTTLER_STATUS_BIT_THM_CORE 4
+#define THROTTLER_STATUS_BIT_THM_GFX5
+#define THROTTLER_STATUS_BIT_THM_SOC6
+#define THROTTLER_STATUS_BIT_TDC_VDD7
+#define THROTTLER_STATUS_BIT_TDC_SOC8
+#define THROTTLER_STATUS_BIT_PROCHOT_CPU9
+#define THROTTLER_STATUS_BIT_PROCHOT_GFX 10
+#define THROTTLER_STATUS_BIT_EDC_CPU 11
+#define THROTTLER_STATUS_BIT_EDC_GFX 12
typedef struct {
uint16_t ClockFrequency[CLOCK_COUNT]; //[MHz]
@@ -180,7 +184,7 @@ typedef struct {
uint16_t Power[2];//[mW] indices: VDDCR_VDD, VDDCR_SOC
uint16_t FanPwm; //[milli]
- uint16_t CurrentSocketPower; //[mW]
+ uint16_t CurrentSocketPower; //[W]
uint16_t CoreFrequency[8];//[MHz]
uint16_t CorePower[8];//[mW]
@@ -193,10 +197,16 @@ typedef struct {
uint16_t ThrottlerStatus;
uint16_t spare;
- uint16_t StapmOriginalLimit; //[mW]
- uint16_t StapmCurrentLimit; //[mW]
- uint16_t ApuPower; //[mW]
- uint16_t dGpuPower; //[mW]
+ uint16_t StapmOriginalLimit; //[W]
+ uint16_t StapmCurrentLimit; //[W]
+ uint16_t ApuPower;//[W]
+ uint16_t dGpuPower; //[W]
+
+ uint16_t VddTdcValue; //[mA]
+ uint16_t SocTdcValue; //[mA]
+ uint16_t VddEdcValue; //[mA]
+ uint16_t SocEdcValue; //[mA]
+ uint16_t reserve[2];
} SmuMetrics_t;
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdkfd: Track GPU memory utilization per process
Track GPU memory usage on a per process basis and report it through
sysfs.
Signed-off-by: Mukul Joshi
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++
drivers/gpu/drm/amd/amdkfd/kfd_priv.h| 7
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 51 ++--
3 files changed, 66 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f8fa03a12add..91d4f45730ae 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -39,6 +39,7 @@
#include "kfd_device_queue_manager.h"
#include "kfd_dbgmgr.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_object.h"
static long kfd_ioctl(struct file *, unsigned int, unsigned long);
static int kfd_open(struct inode *, struct file *);
@@ -1322,6 +1323,9 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file
*filep,
goto err_free;
}
+ /* Update the VRAM usage count */
+ pdd->vram_usage += args->size;
+
mutex_unlock(&p->mutex);
args->handle = MAKE_HANDLE(args->gpu_id, idr_handle);
@@ -1351,6 +1355,7 @@ static int kfd_ioctl_free_memory_of_gpu(struct file
*filep,
void *mem;
struct kfd_dev *dev;
int ret;
+ uint64_t size = 0;
dev = kfd_device_by_id(GET_GPU_ID(args->handle));
if (!dev)
@@ -1372,6 +1377,11 @@ static int kfd_ioctl_free_memory_of_gpu(struct file
*filep,
goto err_unlock;
}
+ if (((struct kgd_mem *)mem)->bo)
+ size = ((struct kgd_mem *)mem)->bo->tbo.mem.size;
+ else
+ pr_debug("BO is NULL\n");
+
ret = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd,
(struct kgd_mem *)mem);
@@ -1382,6 +1392,8 @@ static int kfd_ioctl_free_memory_of_gpu(struct file
*filep,
kfd_process_device_remove_obj_handle(
pdd, GET_IDR_HANDLE(args->handle));
+ pdd->vram_usage -= size;
+
err_unlock:
mutex_unlock(&p->mutex);
return ret;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 43b888b311c7..e7918fc3cee5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -616,6 +616,8 @@ enum kfd_pdd_bound {
PDD_BOUND_SUSPENDED,
};
+#define MAX_VRAM_FILENAME_LEN 11
+
/* Data that is per-process-per device. */
struct kfd_process_device {
/*
@@ -658,6 +660,11 @@ struct kfd_process_device {
/* Is this process/pasid bound to this device? (amd_iommu_bind_pasid) */
enum kfd_pdd_bound bound;
+
+ /* VRAM usage */
+ uint64_t vram_usage;
+ struct attribute attr_vram;
+ char vram_filename[MAX_VRAM_FILENAME_LEN];
};
#define qpd_to_pdd(x) container_of(x, struct kfd_process_device, qpd)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index fe0cd49d4ea7..c7f1e5d89bd9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -79,18 +79,22 @@ static struct kfd_procfs_tree procfs;
static ssize_t kfd_procfs_show(struct kobject *kobj, struct attribute *attr,
char *buffer)
{
- int val = 0;
-
if (strcmp(attr->name, "pasid") == 0) {
struct kfd_process *p = container_of(attr, struct kfd_process,
attr_pasid);
- val = p->pasid;
+
+ return snprintf(buffer, PAGE_SIZE, "%d\n", p->pasid);
+ } else if (strncmp(attr->name, "vram_", 5) == 0) {
+ struct kfd_process_device *pdd = container_of(attr, struct
kfd_process_device,
+ attr_vram);
+ if (pdd)
+ return snprintf(buffer, PAGE_SIZE, "%llu\n",
pdd->vram_usage);
} else {
pr_err("Invalid attribute");
return -EINVAL;
}
- return snprintf(buffer, PAGE_SIZE, "%d\n", val);
+ return 0;
}
static void kfd_procfs_kobj_release(struct kobject *kobj)
@@ -206,6 +210,34 @@ int kfd_procfs_add_queue(struct queue *q)
return 0;
}
+int kfd_procfs_add_vram_usage(struct kfd_process *p)
+{
+ int ret = 0;
+ struct kfd_process_device *pdd;
+
+ if (!p)
+ return -EINVAL;
+
+ if (!p->kobj)
+ return -EFAULT;
+
+ /* Create proc//vram_ file for each GPU */
+ list_for_each_entry(pdd, &p->per_device_data, per_device_list) {
+ snprintf(pdd->vram_filename, MAX_VRAM_FILENAME_LEN, "vram_%u",
+pdd->dev->id);
+ pdd->attr_vram.name = pdd->vram_filename;
+ pdd->attr_vram.mode = KFD_SYSFS_FILE_MODE;
+ sysfs_attr_init(&pdd->attr_vram);
+ ret = sysfs_cre
Re:Re: [PATCH] drm/amdgpu: Remove an unnecessary condition check in reserve_bo_and_cond_vms()
发件人：Markus Elfring
发送日期：2020-04-19 02:18:06
收件人：Bernard Zhao ,Alex Deucher
,"Christian König"
,amd-gfx@lists.freedesktop.org,dri-de...@lists.freedesktop.org
抄送人："Felix Kühling"
,linux-ker...@vger.kernel.org,opensource.ker...@vivo.com,Chunming
Zhou ,Daniel Vetter ,David Airlie
主题：Re: [PATCH] drm/amdgpu: Remove an unnecessary condition check in
reserve_bo_and_cond_vms()>> There is no need to if check again,
>
>Thanks for this information.
>
>* Should the function name be mentioned in this change description?
>
>* Would you like to adjust the patch subject?
>
>
>> maybe we could merge into the above else branch.
>
>I suggest to reconsider this wording.
>
>
>…
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -735,10 +735,8 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
>…
>
>I propose to take further coding style aspects into account.
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=90280eaa88ac1a9140dc759941123530d5545bb6#n191
>
>Possible refactoring:
> if (ret) {
> pr_err(…);
> …
> } else {
> ctx->reserved = true;
> }
>
>
>How do you think about to add the tag “Fixes”?
>
Sure, I will modify the code and adjust this patch subject. I will submit it
again.
>Regards,
>Markus
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Reduce a lock scope in amdgpu_amdkfd_gpuvm_free_memory_of_gpu()
> Maybe we could reduce the mutex_lock(&mem->lock)`s protected code area, > and noneed to protect pr_debug. I suggest to improve the commit message. Would you like to adjust the patch subject? Do you imagine that data synchronisation should evolve in other ways? Regards, Markus ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] amdgpu_cs_bo_handles_chunk, remove check info NULL branch
kvfree ensure that the null pointer will do nothing. If addr is NULL, first is_vmalloc_addr will not enter, and kfree function just return when addr is NULL. There will be no risk here. Signed-off-by: Bernard Zhao --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index af91627b..814bd5d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -98,8 +98,7 @@ static int amdgpu_cs_bo_handles_chunk(struct amdgpu_cs_parser *p, return 0; error_free: - if (info) - kvfree(info); + kvfree(info); return r; } -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Remove an unnecessary null pointer check in amdgpu_cs_bo_handles_chunk()
> kvfree ensure that the null pointer will do nothing. I suggest to improve the commit message. Would you like to adjust the patch subject? Are you looking for further questionable condition checks? … > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -98,8 +98,7 @@ static int amdgpu_cs_bo_handles_chunk(struct > amdgpu_cs_parser *p, … > + kvfree(info); > > return r; > } How do you think about to omit a blank line behind the function call at this source code place? Regards, Markus ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] amdgpu_connector_set_property, fix error branch not return errno
The "if(!encoder)" branch return the same value 0 of the success branch, maybe return -EINVAL is more better. Signed-off-by: Bernard Zhao w --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index f355d9a..1f8c6b4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -474,12 +474,12 @@ static int amdgpu_connector_set_property(struct drm_connector *connector, /* need to find digital encoder on connector */ encoder = amdgpu_connector_find_encoder(connector, DRM_MODE_ENCODER_TMDS); if (!encoder) - return 0; + return -EINVAL; amdgpu_encoder = to_amdgpu_encoder(encoder); if (!amdgpu_encoder->enc_priv) - return 0; + return -EINVAL; dig = amdgpu_encoder->enc_priv; new_coherent_mode = val ? true : false; @@ -494,7 +494,7 @@ static int amdgpu_connector_set_property(struct drm_connector *connector, /* need to find digital encoder on connector */ encoder = amdgpu_connector_find_encoder(connector, DRM_MODE_ENCODER_TMDS); if (!encoder) - return 0; + return -EINVAL; amdgpu_encoder = to_amdgpu_encoder(encoder); @@ -509,7 +509,7 @@ static int amdgpu_connector_set_property(struct drm_connector *connector, /* need to find digital encoder on connector */ encoder = amdgpu_connector_find_encoder(connector, DRM_MODE_ENCODER_TMDS); if (!encoder) - return 0; + return -EINVAL; amdgpu_encoder = to_amdgpu_encoder(encoder); @@ -523,7 +523,7 @@ static int amdgpu_connector_set_property(struct drm_connector *connector, /* need to find digital encoder on connector */ encoder = amdgpu_connector_find_encoder(connector, DRM_MODE_ENCODER_TMDS); if (!encoder) - return 0; + return -EINVAL; amdgpu_encoder = to_amdgpu_encoder(encoder); @@ -537,7 +537,7 @@ static int amdgpu_connector_set_property(struct drm_connector *connector, /* need to find digital encoder on connector */ encoder = amdgpu_connector_find_encoder(connector, DRM_MODE_ENCODER_TMDS); if (!encoder) - return 0; + return -EINVAL; amdgpu_encoder = to_amdgpu_encoder(encoder); @@ -551,7 +551,7 @@ static int amdgpu_connector_set_property(struct drm_connector *connector, /* need to find digital encoder on connector */ encoder = amdgpu_connector_find_encoder(connector, DRM_MODE_ENCODER_TMDS); if (!encoder) - return 0; + return -EINVAL; amdgpu_encoder = to_amdgpu_encoder(encoder); -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Remove an unnecessary condition check in reserve_bo_and_cond_vms()
> There is no need to if check again,
Thanks for this information.
* Should the function name be mentioned in this change description?
* Would you like to adjust the patch subject?
> maybe we could merge into the above else branch.
I suggest to reconsider this wording.
…
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -735,10 +735,8 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
…
I propose to take further coding style aspects into account.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=90280eaa88ac1a9140dc759941123530d5545bb6#n191
Possible refactoring:
if (ret) {
pr_err(…);
…
} else {
ctx->reserved = true;
}
How do you think about to add the tag “Fixes”?
Regards,
Markus
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Return more error codes in amdgpu_connector_set_property()
> The "if(!encoder)" branch return the same value 0 of the success > branch, maybe return -EINVAL is more better. I suggest to improve the commit message. * Would you like to adjust the patch subject? * How do you think about to add the tag “Fixes” because of adjustments for the exception handling? Regards, Markus ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: refine kiq read register
Am 17.04.20 um 17:39 schrieb Felix Kuehling:
Am 2020-04-17 um 2:53 a.m. schrieb Yintian Tao:
According to the current kiq read register method,
there will be race condition when using KIQ to read
register if multiple clients want to read at same time
just like the expample below:
1. client-A start to read REG-0 throguh KIQ
2. client-A poll the seqno-0
3. client-B start to read REG-1 through KIQ
4. client-B poll the seqno-1
5. the kiq complete these two read operation
6. client-A to read the register at the wb buffer and
get REG-1 value
Therefore, directly make kiq write the register value at
the ring buffer then there will be no race condition for
the wb buffer.
v2: supply the read_clock and move the reg_val_offs back
Signed-off-by: Yintian Tao
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 11 --
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 5 +++--
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 14 +---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 14 +---
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 28
6 files changed, 33 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index ea576b4260a4..4e1c0239e561 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -304,10 +304,6 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
spin_lock_init(&kiq->ring_lock);
- r = amdgpu_device_wb_get(adev, &kiq->reg_val_offs);
- if (r)
- return r;
-
ring->adev = NULL;
ring->ring_obj = NULL;
ring->use_doorbell = true;
@@ -331,7 +327,6 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
{
- amdgpu_device_wb_free(ring->adev, ring->adev->gfx.kiq.reg_val_offs);
amdgpu_ring_fini(ring);
}
@@ -675,12 +670,14 @@ uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
uint32_t seq;
struct amdgpu_kiq *kiq = &adev->gfx.kiq;
struct amdgpu_ring *ring = &kiq->ring;
+ uint64_t reg_val_offs = 0;
BUG_ON(!ring->funcs->emit_rreg);
spin_lock_irqsave(&kiq->ring_lock, flags);
amdgpu_ring_alloc(ring, 32);
- amdgpu_ring_emit_rreg(ring, reg);
+ reg_val_offs = (ring->wptr & ring->buf_mask) + 30;
I think that should be (ring->wptr + 30) & ring->buf_mask. Otherwise the
reg_val_offset can be past the end of the ring.
But that still leaves a problem if another command is submitted to the
KIQ before you read the returned reg_val from the ring. Your reg_val can
be overwritten by the new command and you get the wrong result. Or the
command can be overwritten with the reg_val, which will most likely hang
the CP.
You could allocate space on the KIQ ring with a NOP command to prevent
that space from being overwritten by other commands.
Well I was under the assumption that this is actually what is done here.
If that is not the case the patch is a rather clear NAK.
Regards,
Christian.
Regards,
Felix
+ amdgpu_ring_emit_rreg(ring, reg, reg_val_offs);
amdgpu_fence_emit_polling(ring, &seq);
amdgpu_ring_commit(ring);
spin_unlock_irqrestore(&kiq->ring_lock, flags);
@@ -707,7 +704,7 @@ uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev,
uint32_t reg)
if (cnt > MAX_KIQ_REG_TRY)
goto failed_kiq_read;
- return adev->wb.wb[kiq->reg_val_offs];
+ return ring->ring[reg_val_offs];
failed_kiq_read:
pr_err("failed to read reg:%x\n", reg);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 634746829024..ee698f0246d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -103,7 +103,6 @@ struct amdgpu_kiq {
struct amdgpu_ring ring;
struct amdgpu_irq_src irq;
const struct kiq_pm4_funcs *pmf;
- uint32_treg_val_offs;
};
/*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index f61664ee4940..a3d88f2aa9f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -181,7 +181,8 @@ struct amdgpu_ring_funcs {
void (*end_use)(struct amdgpu_ring *ring);
void (*emit_switch_buffer) (struct amdgpu_ring *ring);
void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
- void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
+ void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg,
+ uint64_t reg_val_offs);
void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
uint32_t val, uint32_t mask);
@@
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
Am 17.04.20 um 23:43 schrieb Rodrigo Siqueira:
Hi,
Wyatt made the below patch for fixing this issue. I can apply it on top
of this patchset if you all agree.
[Why]
Current code does not guarantee the correct endianness of memory being
copied to fw, specifically in the case where cpu isn't little endian.
[How]
Windows and Diags are always little endian, so we define a macro that
does nothing. Linux already defines this macro and will do the correct
endianness conversion.
Signed-off-by: Wyatt Wood
Reviewed-by: Harry Wentland
Acked-by: Anthony Koo
Acked-by: Rodrigo Siqueira
---
.../amd/display/modules/power/power_helpers.c | 58 ++-
1 file changed, 31 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
index edb446455f6b..8c37bcc27132 100644
--- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
+++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
@@ -265,9 +265,11 @@ static void fill_backlight_transform_table_v_2_2(struct
dmcu_iram_parameters par
ASSERT(lut_index < params.backlight_lut_array_size);
table->backlight_thresholds[i] = (big_endian) ?
- cpu_to_be16(DIV_ROUNDUP((i * 65536), num_entries)) :
DIV_ROUNDUP((i * 65536), num_entries);
+ cpu_to_be16(DIV_ROUNDUP((i * 65536), num_entries)) :
+ cpu_to_le16(DIV_ROUNDUP((i * 65536), num_entries));
table->backlight_offsets[i] = (big_endian) ?
- cpu_to_be16(params.backlight_lut_array[lut_index]) :
params.backlight_lut_array[lut_index];
+ cpu_to_be16(params.backlight_lut_array[lut_index]) :
+ cpu_to_le16(params.backlight_lut_array[lut_index]);
}
}
@@ -596,7 +598,9 @@ void fill_iram_v_2_3(struct iram_table_v_2_2 *ram_table, struct dmcu_iram_parame
unsigned int set = params.set;
ram_table->flags = 0x0;
- ram_table->min_abm_backlight = (big_endian) ?
cpu_to_be16(params.min_abm_backlight) : params.min_abm_backlight;
+ ram_table->min_abm_backlight = (big_endian) ?
+ cpu_to_be16(params.min_abm_backlight) :
+ cpu_to_le16(params.min_abm_backlight);
for (i = 0; i < NUM_AGGR_LEVEL; i++) {
ram_table->hybrid_factor[i] =
abm_settings[set][i].brightness_gain;
@@ -620,30 +624,30 @@ void fill_iram_v_2_3(struct iram_table_v_2_2 *ram_table,
struct dmcu_iram_parame
ram_table->iir_curve[4] = 0x65;
//Gamma 2.2
- ram_table->crgb_thresh[0] = (big_endian) ? cpu_to_be16(0x127c) : 0x127c;
- ram_table->crgb_thresh[1] = (big_endian) ? cpu_to_be16(0x151b) : 0x151b;
- ram_table->crgb_thresh[2] = (big_endian) ? cpu_to_be16(0x17d5) : 0x17d5;
- ram_table->crgb_thresh[3] = (big_endian) ? cpu_to_be16(0x1a56) : 0x1a56;
- ram_table->crgb_thresh[4] = (big_endian) ? cpu_to_be16(0x1c83) : 0x1c83;
- ram_table->crgb_thresh[5] = (big_endian) ? cpu_to_be16(0x1e72) : 0x1e72;
- ram_table->crgb_thresh[6] = (big_endian) ? cpu_to_be16(0x20f0) : 0x20f0;
- ram_table->crgb_thresh[7] = (big_endian) ? cpu_to_be16(0x232b) : 0x232b;
- ram_table->crgb_offset[0] = (big_endian) ? cpu_to_be16(0x2999) : 0x2999;
- ram_table->crgb_offset[1] = (big_endian) ? cpu_to_be16(0x3999) : 0x3999;
- ram_table->crgb_offset[2] = (big_endian) ? cpu_to_be16(0x4666) : 0x4666;
- ram_table->crgb_offset[3] = (big_endian) ? cpu_to_be16(0x5999) : 0x5999;
- ram_table->crgb_offset[4] = (big_endian) ? cpu_to_be16(0x6333) : 0x6333;
- ram_table->crgb_offset[5] = (big_endian) ? cpu_to_be16(0x7800) : 0x7800;
- ram_table->crgb_offset[6] = (big_endian) ? cpu_to_be16(0x8c00) : 0x8c00;
- ram_table->crgb_offset[7] = (big_endian) ? cpu_to_be16(0xa000) : 0xa000;
- ram_table->crgb_slope[0] = (big_endian) ? cpu_to_be16(0x3609) : 0x3609;
- ram_table->crgb_slope[1] = (big_endian) ? cpu_to_be16(0x2dfa) : 0x2dfa;
- ram_table->crgb_slope[2] = (big_endian) ? cpu_to_be16(0x27ea) : 0x27ea;
- ram_table->crgb_slope[3] = (big_endian) ? cpu_to_be16(0x235d) : 0x235d;
- ram_table->crgb_slope[4] = (big_endian) ? cpu_to_be16(0x2042) : 0x2042;
- ram_table->crgb_slope[5] = (big_endian) ? cpu_to_be16(0x1dc3) : 0x1dc3;
- ram_table->crgb_slope[6] = (big_endian) ? cpu_to_be16(0x1b1a) : 0x1b1a;
- ram_table->crgb_slope[7] = (big_endian) ? cpu_to_be16(0x1910) : 0x1910;
+ ram_table->crgb_thresh[0] = (big_endian) ? cpu_to_be16(0x127c) :
cpu_to_le16(0x127c);
+ ram_table->crgb_thresh[1] = (big_endian) ? cpu_to_be16(0x151b) :
cpu_to_le16(0x151b);
+ ram_table->crgb_thresh[2] = (big_endian) ? cpu_to_be16(0x17d5) :
cpu_to_le16(0x17d5);
+ ram_table->crgb_thresh[3] = (big_endian) ? cpu_to_be16(0x1a56) :
cpu_to_le16(0x1a56);
+ ram_table->crgb_thresh[4] = (big_endian) ? cpu_to_
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
Am 17.04.20 um 17:40 schrieb Harry Wentland: On 2020-04-17 8:09 a.m., Christian König wrote: Am 17.04.20 um 12:43 schrieb Michel Dänzer: On 2020-04-17 11:22 a.m., Christian König wrote: Agreed, just wanted to reply as well since I think something is not correctly understood here. The cpu_to_be16() and be16_to_cpu() functions work different depending on which architecture/endianess your are. So they should be a NO-OP on x86 if everything is done right. The *b*e macros aren't NOPs on little endian architectures like x86, they are on big endian architectures. Vice versa for the *l*e macros. Yeah, that's what I meant with "if everything is done right" :) I usually can't remember what does what with those functions. Christian. I think key here is that dmcub FW is little endian, whereas the old dmcu FW was big endian. Hence we had the cpu_to_be conversion here for the old dmcu. Now it looks like we want to reuse the same function for dmcub calls and hence need to ensure we're not converting values to big-endian. The big_endian parameter is specifying the endianness of the FW. The right approach would be to do cpu_to_be for dmcu and cpu_to_le for dmcub. Thanks for the explanation, that makes it much more clear what should happen here. Christian. Harry ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: replace DRM prefix with PCI device info for gfx/mmhub
[AMD Public Use]
Reviewed-by: Guchun Chen
Regards,
Guchun
-Original Message-
From: Dennis Li
Sent: Saturday, April 18, 2020 12:11 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
; Zhou1, Tao ; Zhang, Hawking
; Chen, Guchun
Cc: Li, Dennis
Subject: [PATCH] drm/amdgpu: replace DRM prefix with PCI device info for
gfx/mmhub
Prefix RAS message printing in gfx/mmhub with PCI device info, which assists
the debug in multiple GPU case.
Change-Id: Iceba7cafd5aac7d0251d9f871503745cc617fba2
Signed-off-by: Dennis Li
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
old mode 100644
new mode 100755
index dce945ef21a5..46351db36922
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
@@ -732,7 +732,8 @@ static int gfx_v9_4_query_utc_edc_status(struct
amdgpu_device *adev,
sec_count = REG_GET_FIELD(data, VML2_WALKER_MEM_ECC_CNTL,
SEC_COUNT);
if (sec_count) {
- DRM_INFO("Instance[%d]: SubBlock %s, SEC %d\n", i,
+ dev_info(adev->dev,
+"Instance[%d]: SubBlock %s, SEC %d\n", i,
vml2_walker_mems[i], sec_count);
err_data->ce_count += sec_count;
}
@@ -740,7 +741,8 @@ static int gfx_v9_4_query_utc_edc_status(struct
amdgpu_device *adev,
ded_count = REG_GET_FIELD(data, VML2_WALKER_MEM_ECC_CNTL,
DED_COUNT);
if (ded_count) {
- DRM_INFO("Instance[%d]: SubBlock %s, DED %d\n", i,
+ dev_info(adev->dev,
+"Instance[%d]: SubBlock %s, DED %d\n", i,
vml2_walker_mems[i], ded_count);
err_data->ue_count += ded_count;
}
@@ -752,14 +754,16 @@ static int gfx_v9_4_query_utc_edc_status(struct
amdgpu_device *adev,
sec_count = REG_GET_FIELD(data, UTCL2_MEM_ECC_CNTL, SEC_COUNT);
if (sec_count) {
- DRM_INFO("Instance[%d]: SubBlock %s, SEC %d\n", i,
+ dev_info(adev->dev,
+"Instance[%d]: SubBlock %s, SEC %d\n", i,
utcl2_router_mems[i], sec_count);
err_data->ce_count += sec_count;
}
ded_count = REG_GET_FIELD(data, UTCL2_MEM_ECC_CNTL, DED_COUNT);
if (ded_count) {
- DRM_INFO("Instance[%d]: SubBlock %s, DED %d\n", i,
+ dev_info(adev->dev,
+"Instance[%d]: SubBlock %s, DED %d\n", i,
utcl2_router_mems[i], ded_count);
err_data->ue_count += ded_count;
}
@@ -772,7 +776,8 @@ static int gfx_v9_4_query_utc_edc_status(struct
amdgpu_device *adev,
sec_count = REG_GET_FIELD(data, ATC_L2_CACHE_2M_DSM_CNTL,
SEC_COUNT);
if (sec_count) {
- DRM_INFO("Instance[%d]: SubBlock %s, SEC %d\n", i,
+ dev_info(adev->dev,
+"Instance[%d]: SubBlock %s, SEC %d\n", i,
atc_l2_cache_2m_mems[i], sec_count);
err_data->ce_count += sec_count;
}
@@ -780,7 +785,8 @@ static int gfx_v9_4_query_utc_edc_status(struct
amdgpu_device *adev,
ded_count = REG_GET_FIELD(data, ATC_L2_CACHE_2M_DSM_CNTL,
DED_COUNT);
if (ded_count) {
- DRM_INFO("Instance[%d]: SubBlock %s, DED %d\n", i,
+ dev_info(adev->dev,
+"Instance[%d]: SubBlock %s, DED %d\n", i,
atc_l2_cache_2m_mems[i], ded_count);
err_data->ue_count += ded_count;
}
@@ -793,7 +799,8 @@ static int gfx_v9_4_query_utc_edc_status(struct
amdgpu_device *adev,
sec_count = REG_GET_FIELD(data, ATC_L2_CACHE_4K_DSM_CNTL,
SEC_COUNT);
if (sec_count) {
- DRM_INFO("Instance[%d]: SubBlock %s, SEC %d\n", i,
+ dev_info(adev->dev,
+"Instance[%d]: SubBlock %s, SEC %d\n", i,
atc_l2_cache_4k_mems[i], sec_count);
err_data->ce_count += sec_count;
}
@@ -801,7 +808,8 @@ static int gfx_v9_4_query_utc_edc_status(struct
amdgpu_device *adev,
ded_count = REG_GET_FIELD(data, ATC_L2_CACHE_4K_DSM_CNTL,
DED_COUNT);
if (ded_count) {
- DR
