[bug report] drm/amd/display: wait for fence without holding reservation lock
Hello Christian König,
The patch 2fac0f53fe59: "drm/amd/display: wait for fence without
holding reservation lock" from Apr 2, 2019, leads to the following
static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:5338
amdgpu_dm_commit_planes()
warn: 'r' unsigned <= 0
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
5238 static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
5239 struct dc_state *dc_state,
5240 struct drm_device *dev,
5241 struct amdgpu_display_manager *dm,
5242 struct drm_crtc *pcrtc,
5243 bool wait_for_vblank)
5244 {
5245 uint32_t i, r;
^
5246 uint64_t timestamp_ns;
5247 struct drm_plane *plane;
5248 struct drm_plane_state *old_plane_state, *new_plane_state;
5249 struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc);
5250 struct drm_crtc_state *new_pcrtc_state =
5251 drm_atomic_get_new_crtc_state(state, pcrtc);
5252 struct dm_crtc_state *acrtc_state =
to_dm_crtc_state(new_pcrtc_state);
5253 struct dm_crtc_state *dm_old_crtc_state =
5254
to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc));
5255 int planes_count = 0, vpos, hpos;
5256 unsigned long flags;
5257 struct amdgpu_bo *abo;
5258 uint64_t tiling_flags;
5259 uint32_t target_vblank, last_flip_vblank;
5260 bool vrr_active = amdgpu_dm_vrr_active(acrtc_state);
5261 bool pflip_present = false;
5262 struct {
5263 struct dc_surface_update surface_updates[MAX_SURFACES];
5264 struct dc_plane_info plane_infos[MAX_SURFACES];
5265 struct dc_scaling_info scaling_infos[MAX_SURFACES];
5266 struct dc_flip_addrs flip_addrs[MAX_SURFACES];
5267 struct dc_stream_update stream_update;
5268 } *bundle;
5269
5270 bundle = kzalloc(sizeof(*bundle), GFP_KERNEL);
5271
5272 if (!bundle) {
5273 dm_error("Failed to allocate update bundle\n");
5274 goto cleanup;
5275 }
5276
5277 /*
5278 * Disable the cursor first if we're disabling all the planes.
5279 * It'll remain on the screen after the planes are re-enabled
5280 * if we don't.
5281 */
5282 if (acrtc_state->active_planes == 0)
5283 amdgpu_dm_commit_cursors(state);
5284
5285 /* update planes when needed */
5286 for_each_oldnew_plane_in_state(state, plane, old_plane_state,
new_plane_state, i) {
5287 struct drm_crtc *crtc = new_plane_state->crtc;
5288 struct drm_crtc_state *new_crtc_state;
5289 struct drm_framebuffer *fb = new_plane_state->fb;
5290 bool plane_needs_flip;
5291 struct dc_plane_state *dc_plane;
5292 struct dm_plane_state *dm_new_plane_state =
to_dm_plane_state(new_plane_state);
5293
5294 /* Cursor plane is handled after stream updates */
5295 if (plane->type == DRM_PLANE_TYPE_CURSOR)
5296 continue;
5297
5298 if (!fb || !crtc || pcrtc != crtc)
5299 continue;
5300
5301 new_crtc_state = drm_atomic_get_new_crtc_state(state,
crtc);
5302 if (!new_crtc_state->active)
5303 continue;
5304
5305 dc_plane = dm_new_plane_state->dc_state;
5306
5307 bundle->surface_updates[planes_count].surface =
dc_plane;
5308 if (new_pcrtc_state->color_mgmt_changed) {
5309 bundle->surface_updates[planes_count].gamma =
dc_plane->gamma_correction;
5310
bundle->surface_updates[planes_count].in_transfer_func =
dc_plane->in_transfer_func;
5311 }
5312
5313 fill_dc_scaling_info(new_plane_state,
5314
>scaling_infos[planes_count]);
5315
5316 bundle->surface_updates[planes_count].scaling_info =
5317 >scaling_infos[planes_count];
5318
5319 plane_needs_flip = old_plane_state->fb &&
new_plane_state->fb;
5320
5321 pflip_present = pflip_present || plane_needs_flip;
5322
5323 if (!plane_needs_flip) {
5324 planes_count += 1;
5325 continue;
Re: [PATCH 1/2] drm/amdgpu: Fix CIK references in gmc_v8
Am 01.05.19 um 14:31 schrieb Russell, Kent:
gmc_v8 is for VI, not CIK, so fix those references
Change-Id: Ifa46212fbeadbec7e73ba28d02e97339ffcfb5d1
Signed-off-by: Kent Russell
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 2880a145450a..6769989906bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -289,7 +289,7 @@ static int gmc_v8_0_init_microcode(struct amdgpu_device
*adev)
*
* @adev: amdgpu_device pointer
*
- * Load the GDDR MC ucode into the hw (CIK).
+ * Load the GDDR MC ucode into the hw (VI).
* Returns 0 on success, error on failure.
*/
static int gmc_v8_0_tonga_mc_load_microcode(struct amdgpu_device *adev)
@@ -443,7 +443,7 @@ static void gmc_v8_0_vram_gtt_location(struct amdgpu_device
*adev,
* @adev: amdgpu_device pointer
*
* Set the location of vram, gart, and AGP in the GPU's
- * physical address space (CIK).
+ * physical address space (VI).
*/
static void gmc_v8_0_mc_program(struct amdgpu_device *adev)
{
@@ -515,7 +515,7 @@ static void gmc_v8_0_mc_program(struct amdgpu_device *adev)
* @adev: amdgpu_device pointer
*
* Look up the amount of vram, vram width, and decide how to place
- * vram and gart within the GPU's physical address space (CIK).
+ * vram and gart within the GPU's physical address space (VI).
* Returns 0 for success.
*/
static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
@@ -630,7 +630,7 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
* @adev: amdgpu_device pointer
* @vmid: vm instance to flush
*
- * Flush the TLB for the requested page table (CIK).
+ * Flush the TLB for the requested page table (VI).
*/
static void gmc_v8_0_flush_gpu_tlb(struct amdgpu_device *adev,
uint32_t vmid, uint32_t flush_type)
@@ -800,7 +800,7 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev,
bool enable)
* This sets up the TLBs, programs the page tables for VMID0,
* sets up the hw for VMIDs 1-15 which are allocated on
* demand, and sets up the global locations for the LDS, GDS,
- * and GPUVM for FSA64 clients (CIK).
+ * and GPUVM for FSA64 clients (VI).
* Returns 0 for success, errors for failure.
*/
static int gmc_v8_0_gart_enable(struct amdgpu_device *adev)
@@ -948,7 +948,7 @@ static int gmc_v8_0_gart_init(struct amdgpu_device *adev)
*
* @adev: amdgpu_device pointer
*
- * This disables all VM page table (CIK).
+ * This disables all VM page table (VI).
*/
static void gmc_v8_0_gart_disable(struct amdgpu_device *adev)
{
@@ -978,7 +978,7 @@ static void gmc_v8_0_gart_disable(struct amdgpu_device
*adev)
* @status: VM_CONTEXT1_PROTECTION_FAULT_STATUS register value
* @addr: VM_CONTEXT1_PROTECTION_FAULT_ADDR register value
*
- * Print human readable fault information (CIK).
+ * Print human readable fault information (VI).
*/
static void gmc_v8_0_vm_decode_fault(struct amdgpu_device *adev, u32 status,
u32 addr, u32 mc_client, unsigned pasid)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Use int for signed error code checks in commit planes
On 5/2/19 9:09 AM, Christian König wrote:
>
> Am 02.05.19 um 15:08 schrieb Christian König:
>> Am 02.05.19 um 15:03 schrieb Nicholas Kazlauskas:
>>> [Why]
>>>
>>> The type of 'r' is uint32_t and the return codes for both:
>>>
>>> - reservation_object_wait_timeout_rcu
>>> - amdgpu_bo_reserve
>>>
>>> ...are signed. While it works for the latter since the check is
>>> done on != 0 it doesn't work for the former since we check <= 0.
>>>
>>> [How]
>>>
>>> Make 'r' an int in commit planes so we're not doing any unsigned/signed
>>> conversion here in the first place.
>>>
>>> Reported-by: Dan Carpenter
>>> Signed-off-by: Nicholas Kazlauskas
>>
>> Reviewed-by: Christian König
>
> Oh, wait a second. Shouldn't this even be a long instead of an int?
>
> Christian.
To be fully correct on all architectures I suppose that's true. I'll
repost it with that fixed. Thanks.
Nicholas Kazlauskas
>
>>
>>> ---
>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index becd8cb3aab6..722f863ce4a4 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -5332,7 +5332,7 @@ static void amdgpu_dm_commit_planes(struct
>>> drm_atomic_state *state,
>>> struct drm_crtc *pcrtc,
>>> bool wait_for_vblank)
>>> {
>>> - uint32_t i, r;
>>> + uint32_t i;
>>> uint64_t timestamp_ns;
>>> struct drm_plane *plane;
>>> struct drm_plane_state *old_plane_state, *new_plane_state;
>>> @@ -5342,7 +5342,7 @@ static void amdgpu_dm_commit_planes(struct
>>> drm_atomic_state *state,
>>> struct dm_crtc_state *acrtc_state =
>>> to_dm_crtc_state(new_pcrtc_state);
>>> struct dm_crtc_state *dm_old_crtc_state =
>>> to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc));
>>> - int planes_count = 0, vpos, hpos;
>>> + int r, planes_count = 0, vpos, hpos;
>>> unsigned long flags;
>>> struct amdgpu_bo *abo;
>>> uint64_t tiling_flags;
>>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [bug report] drm/amd/display: wait for fence without holding reservation lock
On 5/2/19 4:04 AM, Dan Carpenter wrote:
> [CAUTION: External Email]
>
> Hello Christian König,
>
> The patch 2fac0f53fe59: "drm/amd/display: wait for fence without
> holding reservation lock" from Apr 2, 2019, leads to the following
> static checker warning:
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:5338
> amdgpu_dm_commit_planes()
> warn: 'r' unsigned <= 0
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
>5238 static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>5239 struct dc_state *dc_state,
>5240 struct drm_device *dev,
>5241 struct amdgpu_display_manager
> *dm,
>5242 struct drm_crtc *pcrtc,
>5243 bool wait_for_vblank)
>5244 {
>5245 uint32_t i, r;
> ^
>
>5246 uint64_t timestamp_ns;
>5247 struct drm_plane *plane;
>5248 struct drm_plane_state *old_plane_state, *new_plane_state;
>5249 struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc);
>5250 struct drm_crtc_state *new_pcrtc_state =
>5251 drm_atomic_get_new_crtc_state(state, pcrtc);
>5252 struct dm_crtc_state *acrtc_state =
> to_dm_crtc_state(new_pcrtc_state);
>5253 struct dm_crtc_state *dm_old_crtc_state =
>5254
> to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc));
>5255 int planes_count = 0, vpos, hpos;
>5256 unsigned long flags;
>5257 struct amdgpu_bo *abo;
>5258 uint64_t tiling_flags;
>5259 uint32_t target_vblank, last_flip_vblank;
>5260 bool vrr_active = amdgpu_dm_vrr_active(acrtc_state);
>5261 bool pflip_present = false;
>5262 struct {
>5263 struct dc_surface_update
> surface_updates[MAX_SURFACES];
>5264 struct dc_plane_info plane_infos[MAX_SURFACES];
>5265 struct dc_scaling_info scaling_infos[MAX_SURFACES];
>5266 struct dc_flip_addrs flip_addrs[MAX_SURFACES];
>5267 struct dc_stream_update stream_update;
>5268 } *bundle;
>5269
>5270 bundle = kzalloc(sizeof(*bundle), GFP_KERNEL);
>5271
>5272 if (!bundle) {
>5273 dm_error("Failed to allocate update bundle\n");
>5274 goto cleanup;
>5275 }
>5276
>5277 /*
>5278 * Disable the cursor first if we're disabling all the
> planes.
>5279 * It'll remain on the screen after the planes are re-enabled
>5280 * if we don't.
>5281 */
>5282 if (acrtc_state->active_planes == 0)
>5283 amdgpu_dm_commit_cursors(state);
>5284
>5285 /* update planes when needed */
>5286 for_each_oldnew_plane_in_state(state, plane,
> old_plane_state, new_plane_state, i) {
>5287 struct drm_crtc *crtc = new_plane_state->crtc;
>5288 struct drm_crtc_state *new_crtc_state;
>5289 struct drm_framebuffer *fb = new_plane_state->fb;
>5290 bool plane_needs_flip;
>5291 struct dc_plane_state *dc_plane;
>5292 struct dm_plane_state *dm_new_plane_state =
> to_dm_plane_state(new_plane_state);
>5293
>5294 /* Cursor plane is handled after stream updates */
>5295 if (plane->type == DRM_PLANE_TYPE_CURSOR)
>5296 continue;
>5297
>5298 if (!fb || !crtc || pcrtc != crtc)
>5299 continue;
>5300
>5301 new_crtc_state =
> drm_atomic_get_new_crtc_state(state, crtc);
>5302 if (!new_crtc_state->active)
>5303 continue;
>5304
>5305 dc_plane = dm_new_plane_state->dc_state;
>5306
>5307 bundle->surface_updates[planes_count].surface =
> dc_plane;
>5308 if (new_pcrtc_state->color_mgmt_changed) {
>5309 bundle->surface_updates[planes_count].gamma
> = dc_plane->gamma_correction;
>5310
> bundle->surface_updates[planes_count].in_transfer_func =
> dc_plane->in_transfer_func;
>5311 }
>5312
>5313 fill_dc_scaling_info(new_plane_state,
>5314
> >scaling_infos[planes_count]);
>5315
>5316 bundle->surface_updates[planes_count].scaling_info =
>5317
[PATCH] drm/amdgpu: Use FW addr returned by PSP for VF MM
One Vega10 SR-IOV VF, the FW address returned by PSP should be
set into the init table, while not the original BO mc address.
otherwise, UVD and VCE IB test will fail under Vega10 SR-IOV
reference:
commit bfcea5204287 ("drm/amdgpu:change VEGA booting with firmware
loaded by PSP")
commit aa5873dca463 ("drm/amdgpu: Change VCE booting with firmware
loaded by PSP")
Signed-off-by: Trigger Huang
---
drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 16 ++--
drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 17 +++--
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
index dc461df..2191d3d 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
@@ -787,10 +787,13 @@ static int uvd_v7_0_sriov_start(struct amdgpu_device
*adev)
0x,
0x0004);
/* mc resume*/
if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
-
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
mmUVD_LMI_VCPU_CACHE_64BIT_BAR_LOW),
-
lower_32_bits(adev->firmware.ucode[AMDGPU_UCODE_ID_UVD].mc_addr));
-
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
mmUVD_LMI_VCPU_CACHE_64BIT_BAR_HIGH),
-
upper_32_bits(adev->firmware.ucode[AMDGPU_UCODE_ID_UVD].mc_addr));
+
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
+
mmUVD_LMI_VCPU_CACHE_64BIT_BAR_LOW),
+
adev->firmware.ucode[AMDGPU_UCODE_ID_UVD].tmr_mc_addr_lo);
+
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
+
mmUVD_LMI_VCPU_CACHE_64BIT_BAR_HIGH),
+
adev->firmware.ucode[AMDGPU_UCODE_ID_UVD].tmr_mc_addr_hi);
+
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, 0, mmUVD_VCPU_CACHE_OFFSET0),
0);
offset = 0;
} else {
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
mmUVD_LMI_VCPU_CACHE_64BIT_BAR_LOW),
@@ -798,10 +801,11 @@ static int uvd_v7_0_sriov_start(struct amdgpu_device
*adev)
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
mmUVD_LMI_VCPU_CACHE_64BIT_BAR_HIGH),
upper_32_bits(adev->uvd.inst[i].gpu_addr));
offset = size;
+
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, 0, mmUVD_VCPU_CACHE_OFFSET0),
+
AMDGPU_UVD_FIRMWARE_OFFSET >> 3);
+
}
- MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
mmUVD_VCPU_CACHE_OFFSET0),
- AMDGPU_UVD_FIRMWARE_OFFSET
>> 3);
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
mmUVD_VCPU_CACHE_SIZE0), size);
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(UVD, i,
mmUVD_LMI_VCPU_CACHE1_64BIT_BAR_LOW),
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
index f3f5938..c0ec279 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
@@ -244,13 +244,18 @@ static int vce_v4_0_sriov_start(struct amdgpu_device
*adev)
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
mmVCE_LMI_SWAP_CNTL1), 0);
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
mmVCE_LMI_VM_CTRL), 0);
+ offset = AMDGPU_VCE_FIRMWARE_OFFSET;
if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
+ uint32_t low =
adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].tmr_mc_addr_lo;
+ uint32_t hi =
adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].tmr_mc_addr_hi;
+ uint64_t tmr_mc_addr = (uint64_t)(hi) << 32 | low;
+
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
-
mmVCE_LMI_VCPU_CACHE_40BIT_BAR0),
-
adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].mc_addr >> 8);
+
mmVCE_LMI_VCPU_CACHE_40BIT_BAR0), tmr_mc_addr >> 8);
MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
mmVCE_LMI_VCPU_CACHE_64BIT_BAR0),
-
Re: [PATCH] drm/amd/display: Use int for signed error code checks in commit planes
Am 02.05.19 um 15:08 schrieb Christian König:
Am 02.05.19 um 15:03 schrieb Nicholas Kazlauskas:
[Why]
The type of 'r' is uint32_t and the return codes for both:
- reservation_object_wait_timeout_rcu
- amdgpu_bo_reserve
...are signed. While it works for the latter since the check is
done on != 0 it doesn't work for the former since we check <= 0.
[How]
Make 'r' an int in commit planes so we're not doing any unsigned/signed
conversion here in the first place.
Reported-by: Dan Carpenter
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Christian König
Oh, wait a second. Shouldn't this even be a long instead of an int?
Christian.
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index becd8cb3aab6..722f863ce4a4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5332,7 +5332,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct drm_crtc *pcrtc,
bool wait_for_vblank)
{
- uint32_t i, r;
+ uint32_t i;
uint64_t timestamp_ns;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
@@ -5342,7 +5342,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct dm_crtc_state *acrtc_state =
to_dm_crtc_state(new_pcrtc_state);
struct dm_crtc_state *dm_old_crtc_state =
to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc));
- int planes_count = 0, vpos, hpos;
+ int r, planes_count = 0, vpos, hpos;
unsigned long flags;
struct amdgpu_bo *abo;
uint64_t tiling_flags;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Use int for signed error code checks in commit planes
Am 02.05.19 um 15:03 schrieb Nicholas Kazlauskas:
[Why]
The type of 'r' is uint32_t and the return codes for both:
- reservation_object_wait_timeout_rcu
- amdgpu_bo_reserve
...are signed. While it works for the latter since the check is
done on != 0 it doesn't work for the former since we check <= 0.
[How]
Make 'r' an int in commit planes so we're not doing any unsigned/signed
conversion here in the first place.
Reported-by: Dan Carpenter
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index becd8cb3aab6..722f863ce4a4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5332,7 +5332,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct drm_crtc *pcrtc,
bool wait_for_vblank)
{
- uint32_t i, r;
+ uint32_t i;
uint64_t timestamp_ns;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
@@ -5342,7 +5342,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state);
struct dm_crtc_state *dm_old_crtc_state =
to_dm_crtc_state(drm_atomic_get_old_crtc_state(state,
pcrtc));
- int planes_count = 0, vpos, hpos;
+ int r, planes_count = 0, vpos, hpos;
unsigned long flags;
struct amdgpu_bo *abo;
uint64_t tiling_flags;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm/amd/display: Use long for signed error code checks in commit planes
Am 02.05.19 um 15:14 schrieb Nicholas Kazlauskas:
[Why]
The type of 'r' is uint32_t and the return codes for both:
- reservation_object_wait_timeout_rcu
- amdgpu_bo_reserve
...are signed. While it works for the latter since the check is
done on != 0 it doesn't work for the former since we check <= 0.
[How]
Make 'r' a long in commit planes so we're not doing any unsigned/signed
conversion here in the first place.
v2: use long instead of int (Christian)
Cc: Christian König
Reported-by: Dan Carpenter
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index becd8cb3aab6..ac22f7351a42 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5332,7 +5332,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct drm_crtc *pcrtc,
bool wait_for_vblank)
{
- uint32_t i, r;
+ uint32_t i;
uint64_t timestamp_ns;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
@@ -5343,6 +5343,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct dm_crtc_state *dm_old_crtc_state =
to_dm_crtc_state(drm_atomic_get_old_crtc_state(state,
pcrtc));
int planes_count = 0, vpos, hpos;
+ long r;
unsigned long flags;
struct amdgpu_bo *abo;
uint64_t tiling_flags;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/amdgpu: Add MEM_LOAD to amdgpu_pm_info debugfs file
Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index 5e2d039e09ad..e0789f0f2670 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -2955,6 +2955,10 @@ static int amdgpu_debugfs_pm_info_pp(struct seq_file *m, struct amdgpu_device *a /* GPU Load */ if (!amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_LOAD, (void *), )) seq_printf(m, "GPU Load: %u %%\n", value); + /* MEM Load */ + if (!amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_MEM_LOAD, (void *), )) + seq_printf(m, "MEM Load: %u %%\n", value); + seq_printf(m, "\n"); /* SMC feature mask */ -- 2.20.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: Use int for signed error code checks in commit planes
[Why]
The type of 'r' is uint32_t and the return codes for both:
- reservation_object_wait_timeout_rcu
- amdgpu_bo_reserve
...are signed. While it works for the latter since the check is
done on != 0 it doesn't work for the former since we check <= 0.
[How]
Make 'r' an int in commit planes so we're not doing any unsigned/signed
conversion here in the first place.
Reported-by: Dan Carpenter
Signed-off-by: Nicholas Kazlauskas
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index becd8cb3aab6..722f863ce4a4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5332,7 +5332,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct drm_crtc *pcrtc,
bool wait_for_vblank)
{
- uint32_t i, r;
+ uint32_t i;
uint64_t timestamp_ns;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
@@ -5342,7 +5342,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state);
struct dm_crtc_state *dm_old_crtc_state =
to_dm_crtc_state(drm_atomic_get_old_crtc_state(state,
pcrtc));
- int planes_count = 0, vpos, hpos;
+ int r, planes_count = 0, vpos, hpos;
unsigned long flags;
struct amdgpu_bo *abo;
uint64_t tiling_flags;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 27/27] drm/amdgpu: Fix GTT size calculation
Am 30.04.19 um 19:25 schrieb Kuehling, Felix: > [SNIP] To sum it up the requirement of using almost all system memory by a GPU is simply not possible upstream and even in any production system rather questionable. >>> It should be doable with userptr, which now uses unpinned pages through >>> HMM. Currently the GTT limit affects the largest possible userptr >>> allocation, though not the total sum of all userptr allocations. Maybe >>> making userptr completely independent of GTT size would be an easier >>> problem to tackle. Then I can leave the GTT limit alone. >> Well this way we would only avoid the symptoms, but not the real problem. > It allocates pages in user mode rather than kernel mode. That means, OOM > situations take a completely different code path. Before running out of > memory completely, triggering the OOM killer, the kernel would start > swapping pages, which would trigger the MMU notifier to stop the user > mode queues or invalidate GPU page table entries, and allow the pages to > be swapped out. Well it at least removes the extra layer in TTM we have here. But what I meant is that it still doesn't fix our underlying problem of stopping the hardware immediately. The only real solution I can see is to be able to reliable kill shaders in an OOM situation. >>> Well, we can in fact preempt our compute shaders with low latency. >>> Killing a KFD process will do exactly that. >> I've taken a look at that thing as well and to be honest it is not even >> remotely sufficient. >> >> We need something which stops the hardware *immediately* from accessing >> system memory, and not wait for the SQ to kill all waves, flush caches >> etc... > It's apparently sufficient to use in our MMU notifier. There is also a > way to disable the grace period that allows short waves to complete > before being preempted, though we're not using that at the moment. > > >> One possibility I'm playing around with for a while is to replace the >> root PD for the VMIDs in question on the fly. E.g. we just let it point >> to some dummy which redirects everything into nirvana. > Even that's not sufficient. You'll also need to free the pages > immediately. For KFD processes, cleaning up of memory is done in a > worker thread that gets kicked off by a release MMU notifier when the > process' mm_struct is taken down. Yeah, that worker is what I meant with that this whole thing is not sufficient. BTW: How does that still work with HMM? I mean HMM doesn't take a reference to the pages any more. But let me say it differently: When we want the OOM killer to work correctly we need to have a short cut path which doesn't takes any locks or allocates memory. What we can do is: Write some registers and then maybe busy wait for the TLB flush to complete. What we can't do is: Waiting for the SQ to kill waves, waiting for fences etc... > Then there is still TTM's delayed freeing of BOs that waits for fences. > So you'd need to signal all the BO fences to allow them to be released. Actually that doesn't apply in the critical code path. In this situation TTM tries to free things up it doesn't need to wait for immediately. What we are missing here is something like a kill interface for fences maybe... > TBH, I don't understand why waiting is not an option, if the alternative > is a kernel panic. If your OOM killer kicks in, your system is basically > dead. Waiting for a fraction of a second to let a GPU finish its memory > access should be a small price to pay in that situation. Oh, yeah that is really good point as well. I think that this restriction was created to make sure that the OOM killer always makes progress and doesn't waits for things like network congestion. Now the Linux MM is not really made for long term I/O mappings anyway. And that was also recently a topic on the lists in the context of HMM (there is a LWN summary about it). Probably worth bringing that discussion up once more. Christian. > > Regards, > Felix > > >> But implementing this is easier said than done... >> >> Regards, >> Christian. >> ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2] drm/amd/display: Use long for signed error code checks in commit planes
[Why]
The type of 'r' is uint32_t and the return codes for both:
- reservation_object_wait_timeout_rcu
- amdgpu_bo_reserve
...are signed. While it works for the latter since the check is
done on != 0 it doesn't work for the former since we check <= 0.
[How]
Make 'r' a long in commit planes so we're not doing any unsigned/signed
conversion here in the first place.
v2: use long instead of int (Christian)
Cc: Christian König
Reported-by: Dan Carpenter
Signed-off-by: Nicholas Kazlauskas
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index becd8cb3aab6..ac22f7351a42 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5332,7 +5332,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct drm_crtc *pcrtc,
bool wait_for_vblank)
{
- uint32_t i, r;
+ uint32_t i;
uint64_t timestamp_ns;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
@@ -5343,6 +5343,7 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
struct dm_crtc_state *dm_old_crtc_state =
to_dm_crtc_state(drm_atomic_get_old_crtc_state(state,
pcrtc));
int planes_count = 0, vpos, hpos;
+ long r;
unsigned long flags;
struct amdgpu_bo *abo;
uint64_t tiling_flags;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[pull] amdgpu drm-next-5.2
Hi Dave, Daniel, Fixes for 5.2: - SR-IOV fixes - Raven flickering fix - Misc spelling fixes - Vega20 power fixes - Freesync improvements - DC fixes The following changes since commit f55be0be5b7296e73f1634e2839a1953dc12d11e: drm/amd/display: Add profiling tools for bandwidth validation (2019-04-15 00:22:19 -0500) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux drm-next-5.2 for you to fetch changes up to b0fc850fd95f8ecceb601bbb40624da0a8c220a0: drm/amdgpu: power down the Vega20 VCE engine on request (2019-04-29 14:59:58 -0500) Amber Lin (1): drm/amdgpu: get_fw_version isn't ASIC specific Andrey Grodzovsky (2): drm/amdgpu: Change VRAM lost print from ERR to INF drm/amd/display: Use a reasonable timeout for framebuffer fence waits Anthony Koo (3): drm/amd/display: Allow system to enter stutter on init drm/amd/display: Send DMCU messages only if FW loaded drm/amd/display: Fix eDP Black screen after S4 resume Aric Cyr (1): drm/amd/display: 3.2.27 Charlene Liu (1): drm/amd/display: Add hubp_init entry to hubp vtable Chengming Gui (2): drm/amd/powerplay: add set/get_power_profile_mode for Raven (v2) drm/amd/powerplay: enable UMDPSTATE support on raven2 (v2) Christian König (1): drm/amd/display: wait for fence without holding reservation lock Colin Ian King (3): drm/amdgpu: fix spelling mistake "gateing" -> "gating" drm/amd/amdgpu: fix spelling mistake "recieve" -> "receive" drm/amd/display: fix incorrect null check on pointer Eric Bernstein (1): drm/amd/display: Allow cursor position when plane_res.ipp is NULL Eric Yang (1): drm/amd/display: remove deprecated pplib interface Evan Quan (4): drm/amdgpu: enable Vega20 BACO reset support drm/amdgpu: update Vega20 sdma golden settings drm/amdgpu: expose VCE 4.0 powergate interface drm/amdgpu: power down the Vega20 VCE engine on request John Barberiz (1): drm/amd/display: Refactor dp vendor parsing logic to a function Jun Lei (1): drm/amd/display: add explicit handshake between x86 and DMCU Leo Li (3): drm/amd/include: Add USB_C_TYPE to atom_encoder_cap_defs drm/amd/include: Add HUBPREQ_DEBUG register offsets drm/amdgpu: Check if SW SMU is supported before accessing funcs Likun Gao (1): drm/amdgpu: enable MGCG for PCO Mario Kleiner (2): drm/amd/display: Fix and simplify apply_below_the_range() drm/amd/display: Compensate for pre-DCE12 BTR-VRR hw limitations. (v3) Nicholas Kazlauskas (8): drm/amd/display: Expose support for DRM_FORMAT_RGB565 drm/amd/display: Refactor CRTC interrupt toggling logic drm/amd/display: Disable cursors before disabling planes drm/amd/display: Fix CRC vblank refs when changing interrupts drm/amd/display: Split enabling CRTC interrupts into two passes drm/amd/display: Allow commits with no planes active drm/amd/display: Do VRR transition before enable_crc_interrupts drm/amd/display: Expose DRM_FORMAT_RGB565 on overlay planes Thomas Lim (1): drm/amd/display: Add power down display on boot flag Wenjing Liu (1): drm/amd/display: Add function to copy DC streams Wentao Lou (1): drm/amdgpu: value of amdgpu_sriov_vf cannot be set into F32_POLL_ENABLE Yintian Tao (1): drm/amdgpu: disable DRIVER_ATOMIC under SRIOV Yongqiang Sun (1): drm/amd/display: Refactor watermark programming hersen wu (1): drm/amd/powerplay: raven 4k@60hz dp monitor always flicking shaoyunl (1): drm/powerplay : send SMC message to set XGMI pstate wentalou (1): drm/amdgpu: amdgpu_device_recover_vram got NULL of shadow->parent drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 37 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 14 + drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 61 - drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 61 - drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 54 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 1 + drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 +- drivers/gpu/drm/amd/amdgpu/soc15.c | 9 +- drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 15 +- drivers/gpu/drm/amd/amdkfd/kfd_device.c| 4 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 302 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 + drivers/gpu/drm/amd/display/dc/core/dc_link.c | 71 +++-- drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c |
Re: [PATCH] drm/amd/amdgpu: Add MEM_LOAD to amdgpu_pm_info debugfs file
Reviewed-by: Alex Deucher From: amd-gfx on behalf of StDenis, Tom Sent: Thursday, May 2, 2019 10:22 AM To: amd-gfx@lists.freedesktop.org Cc: StDenis, Tom Subject: [PATCH] drm/amd/amdgpu: Add MEM_LOAD to amdgpu_pm_info debugfs file [CAUTION: External Email] Signed-off-by: Tom St Denis --- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index 5e2d039e09ad..e0789f0f2670 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -2955,6 +2955,10 @@ static int amdgpu_debugfs_pm_info_pp(struct seq_file *m, struct amdgpu_device *a /* GPU Load */ if (!amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_LOAD, (void *), )) seq_printf(m, "GPU Load: %u %%\n", value); + /* MEM Load */ + if (!amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_MEM_LOAD, (void *), )) + seq_printf(m, "MEM Load: %u %%\n", value); + seq_printf(m, "\n"); /* SMC feature mask */ -- 2.20.1 ___ 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
