[pull] amdgpu drm-fixes-5.1
Hi Dave, Daniel, Fixes for 5.1: - Fix for pcie dpm - Powerplay fixes for vega20 - Fix vbios display on reboot if driver display state is retained - Gfx9 resume robustness fix The following changes since commit 0ab925d3690614aa44cd29fb84cdcef03eab97dc: drm/amd/display: Only allow VRR when vrefresh is within supported range (2019-03-21 14:34:59 -0500) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux drm-fixes-5.1 for you to fetch changes up to d939f44d4a7f910755165458da20407d2139f581: drm/amdgpu: remove unnecessary rlc reset function on gfx9 (2019-04-02 16:23:16 -0500) Chengming Gui (1): drm/amd/amdgpu: fix PCIe dpm feature issue (v3) Evan Quan (3): drm/amd/powerplay: add ECC feature bit drm/amd/powerplay: correct data type to avoid overflow drm/amd/powerplay: fix possible hang with 3+ 4K monitors Le Ma (1): drm/amdgpu: remove unnecessary rlc reset function on gfx9 Paul Hsieh (1): drm/amd/display: VBIOS can't be light up HDMI when restart system drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 + drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 -- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 ++ drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 20 ++-- drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h | 1 + drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h | 5 +++-- 6 files changed, 33 insertions(+), 6 deletions(-) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: How to get useful information other than "the whole system locks up"?
On Wed, Apr 3, 2019 at 2:02 PM Alex Deucher wrote: > > On Wed, Apr 3, 2019 at 2:58 AM Braiam wrote: > > > > Hi, > > > > I have a Sapphire Technology Hawaii XT (R9 290X) using amdgpu driver > > with kernel 5.1.0-rc3. > > The issue happens with current 4.19.0 debian testing, 4.20-trunk, > > 5.0.0-trunk and rc2 and 3. > > > > It usually happens when I'm reproducing video, but I haven't figured > > out a way to reproduce it. It > > happened once without reproducing. I'm aware that the support is > > experimental, but radeon > > driver doesn't seems capable of direct rendering on this card dropping > > to llvmepipe. > > Radeon should work out of the box. Maybe something is messed up with > your install? Doubtful, since I used 2200G without issues. Only change was using amdgpu due games being particularly slow. > > > > > I had a ssh server installed in case I could log in while it crashes, > > and the only relevant > > line I found was: > > > > drm:amdgpu job timeout [amdgpu]] **ERROR** ring gfx timeout, signaled > > seq=399919, emitted seq=399921 > > > > But that turned several bug reports which seems to have been fixed and > > the context and symptoms are too different to mine. > > > > You appear to be experiencing a GPU lockup. Unfortunately, there can > be many things that cause it, so it really helps to have a good > reproducer case. You might try a newer version of mesa or llvm. What > does your "reproducing video" work flow use? What apps, APIs are > involved? By "reproducing video" I mean watching videos from either Youtube/Netflix with Firefox or mpv for local files. But note that I've experienced at least one lock up without any video on screen (I don't remember if there was something paused on the background). Using mesa 18.3.4, and according to glxinfo DRM 3.30.0, LLVM 7.0.1. With vulkan, vdpau and va drivers on the same version. I gave up and removed any and every instance of the rocm package, since the module couldn't compile on the kernel. Another detail about the lock up: the screen doesn't freeze but no signal is emitted, so my monitor goes to sleep. Upgraded to mesa 19.0.1 and llvm 8.0.0, although I didn't see anything relevant on the changelogs that could affect my experience. > > Alex > > > I have tried forcing the amdgpu xorg driver with same results (was > > using radeon). > > > > -- > > Braiam > > ___ > > amd-gfx mailing list > > amd-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx -- Braiam ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 20/20] drm/amd/display: extending AUX SW Timeout
From: Martin Leung
[Why]
AUX takes longer to reply when using active DP-DVI dongle on some asics
resulting in up to 2000+ us edid read (timeout).
[How]
1. Adjust AUX poll to match spec
2. Extend the SW timeout. This does not affect normal
operation since we exit the loop as soon as AUX acks.
Signed-off-by: Martin Leung
Reviewed-by: Jun Lei
Acked-by: Joshua Aberback
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 9 ++---
drivers/gpu/drm/amd/display/dc/dce/dce_aux.h | 6 +++---
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
index 937b5cf..bd33c47 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
@@ -190,6 +190,12 @@ static void submit_channel_request(
AUXP_IMPCAL_OVERRIDE_ENABLE, 1,
AUXP_IMPCAL_OVERRIDE_ENABLE, 0);
}
+
+ REG_UPDATE(AUX_INTERRUPT_CONTROL, AUX_SW_DONE_ACK, 1);
+
+ REG_WAIT(AUX_SW_STATUS, AUX_SW_DONE, 0,
+ 10, aux110->timeout_period/10);
+
/* set the delay and the number of bytes to write */
/* The length include
@@ -242,9 +248,6 @@ static void submit_channel_request(
}
}
- REG_UPDATE(AUX_INTERRUPT_CONTROL, AUX_SW_DONE_ACK, 1);
- REG_WAIT(AUX_SW_STATUS, AUX_SW_DONE, 0,
- 10, aux110->timeout_period/10);
REG_UPDATE(AUX_SW_CONTROL, AUX_SW_GO, 1);
}
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
index aab5f0c..ce6a26d 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
@@ -71,11 +71,11 @@ enum { /* This is the timeout as defined in DP 1.2a,
* at most within ~240usec. That means,
* increasing this timeout will not affect normal operation,
* and we'll timeout after
-* SW_AUX_TIMEOUT_PERIOD_MULTIPLIER * AUX_TIMEOUT_PERIOD = 1600usec.
+* SW_AUX_TIMEOUT_PERIOD_MULTIPLIER * AUX_TIMEOUT_PERIOD = 2400usec.
* This timeout is especially important for
-* resume from S3 and CTS.
+* converters, resume from S3, and CTS.
*/
- SW_AUX_TIMEOUT_PERIOD_MULTIPLIER = 4
+ SW_AUX_TIMEOUT_PERIOD_MULTIPLIER = 6
};
struct dce_aux {
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 13/20] drm/amd/display: disable link before changing link settings
From: Anthony Koo
[Why]
If link is already enabled at a different rate (for example 5.4 Gbps)
then calling VBIOS command table to switch to a new rate
(for example 2.7 Gbps) will not take effect.
This can lead to link training failure to occur.
[How]
If the requested link rate is different than the current link rate,
the link must be disabled in order to re-enable at the new
link rate.
In today's logic it is currently only impacting eDP since DP
connection types will always disable the link during display
detection, when initial link verification occurs.
Signed-off-by: Anthony Koo
Reviewed-by: Aric Cyr
Acked-by: Leo Li
Acked-by: Tony Cheng
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 9db5a55..3ef68a2 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1396,6 +1396,15 @@ static enum dc_status enable_link_dp(
/* get link settings for video mode timing */
decide_link_settings(stream, _settings);
+ /* If link settings are different than current and link already enabled
+* then need to disable before programming to new rate.
+*/
+ if (link->link_status.link_active &&
+ (link->cur_link_settings.lane_count != link_settings.lane_count
||
+link->cur_link_settings.link_rate != link_settings.link_rate))
{
+ dp_disable_link_phy(link, pipe_ctx->stream->signal);
+ }
+
pipe_ctx->stream_res.pix_clk_params.requested_sym_clk =
link_settings.link_rate * LINK_RATE_REF_FREQ_IN_KHZ;
state->clk_mgr->funcs->update_clocks(state->clk_mgr, state, false);
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 19/20] drm/amd/display: fix is odm head pipe logic
From: Dmytro Laktyushkin
Simply return true/false, don't iterate up the tree.
Signed-off-by: Dmytro Laktyushkin
Reviewed-by: Nikola Cornij
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index f798fc2..3830e6c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -1305,18 +1305,13 @@ struct pipe_ctx *dc_res_get_odm_bottom_pipe(struct
pipe_ctx *pipe_ctx)
bool dc_res_is_odm_head_pipe(struct pipe_ctx *pipe_ctx)
{
struct pipe_ctx *top_pipe = pipe_ctx->top_pipe;
- bool result = false;
+ if (!top_pipe)
+ return false;
if (top_pipe && top_pipe->stream_res.opp == pipe_ctx->stream_res.opp)
return false;
- while (top_pipe) {
- if (!top_pipe->top_pipe && top_pipe->stream_res.opp !=
pipe_ctx->stream_res.opp)
- result = true;
- top_pipe = top_pipe->top_pipe;
- }
-
- return result;
+ return true;
}
bool dc_remove_plane_from_context(
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 16/20] drm/amd/display: Recreate private_obj->state during S3 resume
From: Leo Li [Why] When entering S3, amdgpu first calls DRM to cache the current atomic state, then commit the 'all-disabled' state. This sets dc->current_state to point at the private atomic object's dm_atomic_state->context, as any regular atomic commit would. Afterwards, amdgpu_dm calls dc_set_power_state() with S3 power state. This invalidates dc->current_state by wiping it to 0, consequently wiping dm_atomic_state->context. During resume, the cached atomic state is restored. When getting the private object however, the dm_atomic_state - containing the wiped context - is duplicated into the atomic state. This causes DC validation to fail during atomic check, as necessary function pointers in dc_state are now NULL. [How] Recreate the private object's dm_atomic_state->context during resume, restoring any static values such as function pointers. A TODO item is added to move static read-only values out of dc_state - they shouldn't be there anyways. Signed-off-by: Leo Li Reviewed-by: Nicholas Kazlauskas --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++ 1 file changed, 7 insertions(+) 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 3588ca7..a436ec3 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -983,9 +983,16 @@ static int dm_resume(void *handle) struct drm_plane *plane; struct drm_plane_state *new_plane_state; struct dm_plane_state *dm_new_plane_state; + struct dm_atomic_state *dm_state = to_dm_atomic_state(dm->atomic_obj.state); enum dc_connection_type new_connection_type = dc_connection_none; int i; + /* Recreate dc_state - DC invalidates it when setting power state to S3. */ + dc_release_state(dm_state->context); + dm_state->context = dc_create_state(dm->dc); + /* TODO: Remove dc_state->dccg, use dc->dccg directly. */ + dc_resource_state_construct(dm->dc, dm_state->context); + /* power on hardware */ dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0); -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 15/20] drm/amd/display: HDR visual confirmation incorrectly reports black color
From: Murton Liu
[Why]
Checking against a TF that is unused causes us to default to black
[How]
Check against PQ instead
Signed-off-by: Murton Liu
Reviewed-by: Aric Cyr
Acked-by: Leo Li
Acked-by: Tony Cheng
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index f659148..c2b93a8 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -1852,7 +1852,7 @@ void dcn10_get_hdr_visual_confirm_color(
switch (top_pipe_ctx->plane_res.scl_data.format) {
case PIXEL_FORMAT_ARGB2101010:
- if (top_pipe_ctx->stream->out_transfer_func->tf ==
TRANSFER_FUNCTION_UNITY) {
+ if (top_pipe_ctx->stream->out_transfer_func->tf ==
TRANSFER_FUNCTION_PQ) {
/* HDR10, ARGB2101010 - set boarder color to red */
color->color_r_cr = color_value;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 17/20] drm/amd/display: Clean up locking in dcn*_apply_ctx_for_surface()
From: Leo Li
[Why]
dcn*_disable_plane() doesn't unlock the pipe anymore, making the extra
lock unnecessary.
In addition - during full plane updates - all necessary pipes should be
locked/unlocked together when modifying hubp to avoid tearing in
pipesplit setups.
[How]
Remove redundant locks, and add function to lock all pipes. If an
interdependent pipe update is required, lock down all pipes. Otherwise,
lock only the top pipe for the updated pipe tree.
Signed-off-by: Leo Li
Reviewed-by: Tony Cheng
Acked-by: Nicholas Kazlauskas
---
.../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 66 +++---
.../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h | 4 ++
2 files changed, 49 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index c2b93a8..dab3706 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -2329,6 +2329,7 @@ static void dcn10_apply_ctx_for_surface(
int i;
struct timing_generator *tg;
bool removed_pipe[4] = { false };
+ bool interdependent_update = false;
struct pipe_ctx *top_pipe_to_program =
find_top_pipe_for_stream(dc, context, stream);
DC_LOGGER_INIT(dc->ctx->logger);
@@ -2338,7 +2339,13 @@ static void dcn10_apply_ctx_for_surface(
tg = top_pipe_to_program->stream_res.tg;
- dcn10_pipe_control_lock(dc, top_pipe_to_program, true);
+ interdependent_update = top_pipe_to_program->plane_state &&
+ top_pipe_to_program->plane_state->update_flags.bits.full_update;
+
+ if (interdependent_update)
+ lock_all_pipes(dc, context, true);
+ else
+ dcn10_pipe_control_lock(dc, top_pipe_to_program, true);
if (num_planes == 0) {
/* OTG blank before remove all front end */
@@ -2358,15 +2365,9 @@ static void dcn10_apply_ctx_for_surface(
*/
if (pipe_ctx->plane_state && !old_pipe_ctx->plane_state) {
if (old_pipe_ctx->stream_res.tg == tg &&
- old_pipe_ctx->plane_res.hubp &&
- old_pipe_ctx->plane_res.hubp->opp_id != 0xf) {
+ old_pipe_ctx->plane_res.hubp &&
+ old_pipe_ctx->plane_res.hubp->opp_id != 0xf)
dcn10_disable_plane(dc, old_pipe_ctx);
- /*
-* power down fe will unlock when calling
reset, need
-* to lock it back here. Messy, need rework.
-*/
-
pipe_ctx->stream_res.tg->funcs->lock(pipe_ctx->stream_res.tg);
- }
}
if ((!pipe_ctx->plane_state ||
@@ -2385,29 +2386,25 @@ static void dcn10_apply_ctx_for_surface(
if (num_planes > 0)
program_all_pipe_in_tree(dc, top_pipe_to_program, context);
- dcn10_pipe_control_lock(dc, top_pipe_to_program, false);
-
- if (top_pipe_to_program->plane_state &&
-
top_pipe_to_program->plane_state->update_flags.bits.full_update)
+ if (interdependent_update)
for (i = 0; i < dc->res_pool->pipe_count; i++) {
struct pipe_ctx *pipe_ctx =
>res_ctx.pipe_ctx[i];
- tg = pipe_ctx->stream_res.tg;
/* Skip inactive pipes and ones already updated */
- if (!pipe_ctx->stream || pipe_ctx->stream == stream
- || !pipe_ctx->plane_state
- || !tg->funcs->is_tg_enabled(tg))
+ if (!pipe_ctx->stream || pipe_ctx->stream == stream ||
+ !pipe_ctx->plane_state ||
!tg->funcs->is_tg_enabled(tg))
continue;
- tg->funcs->lock(tg);
-
pipe_ctx->plane_res.hubp->funcs->hubp_setup_interdependent(
pipe_ctx->plane_res.hubp,
_ctx->dlg_regs,
_ctx->ttu_regs);
-
- tg->funcs->unlock(tg);
}
+ if (interdependent_update)
+ lock_all_pipes(dc, context, false);
+ else
+ dcn10_pipe_control_lock(dc, top_pipe_to_program, false);
+
if (num_planes == 0)
false_optc_underflow_wa(dc, stream, tg);
@@ -2814,6 +2811,33 @@ int get_vupdate_offset_from_vsync(struct pipe_ctx
*pipe_ctx)
return vertical_line_start;
}
+void lock_all_pipes(struct dc *dc,
+ struct dc_state *context,
+ bool lock)
+{
+ struct pipe_ctx *pipe_ctx;
+ struct timing_generator *tg;
+
[PATCH 18/20] drm/amd/display: Pass plane caps into amdgpu_dm_plane_init
From: Nicholas Kazlauskas
[Why]
When deciding to add properties or expose formats on DRM planes we
should be querying the caps for the DC plane it's supposed to represent.
[How]
Pass plane caps down into plane initialization, refactoring overlay
plane initialization to have the overlay plane be represented by
the first overlay capable DC plane.
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Leo Li
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 63 ---
1 file changed, 34 insertions(+), 29 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 a436ec3..cb149a8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -111,7 +111,8 @@ amdgpu_dm_update_connector_after_detect(struct
amdgpu_dm_connector *aconnector);
static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
struct drm_plane *plane,
- unsigned long possible_crtcs);
+ unsigned long possible_crtcs,
+ const struct dc_plane_cap *plane_cap);
static int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
struct drm_plane *plane,
uint32_t link_index);
@@ -1961,7 +1962,8 @@ amdgpu_dm_register_backlight_device(struct
amdgpu_display_manager *dm)
static int initialize_plane(struct amdgpu_display_manager *dm,
struct amdgpu_mode_info *mode_info, int plane_id,
- enum drm_plane_type plane_type)
+ enum drm_plane_type plane_type,
+ const struct dc_plane_cap *plane_cap)
{
struct drm_plane *plane;
unsigned long possible_crtcs;
@@ -1984,7 +1986,7 @@ static int initialize_plane(struct amdgpu_display_manager
*dm,
if (plane_id >= dm->dc->caps.max_streams)
possible_crtcs = 0xff;
- ret = amdgpu_dm_plane_init(dm, plane, possible_crtcs);
+ ret = amdgpu_dm_plane_init(dm, plane, possible_crtcs, plane_cap);
if (ret) {
DRM_ERROR("KMS: Failed to initialize plane\n");
@@ -2037,8 +2039,9 @@ static int amdgpu_dm_initialize_drm_device(struct
amdgpu_device *adev)
struct amdgpu_encoder *aencoder = NULL;
struct amdgpu_mode_info *mode_info = >mode_info;
uint32_t link_cnt;
- int32_t overlay_planes, primary_planes;
+ int32_t primary_planes;
enum dc_connection_type new_connection_type = dc_connection_none;
+ const struct dc_plane_cap *plane;
link_cnt = dm->dc->caps.max_links;
if (amdgpu_dm_mode_config_init(dm->adev)) {
@@ -2046,24 +2049,6 @@ static int amdgpu_dm_initialize_drm_device(struct
amdgpu_device *adev)
return -EINVAL;
}
- /*
-* Determine the number of overlay planes supported.
-* Only support DCN for now, and cap so we don't encourage
-* userspace to use up all the planes.
-*/
- overlay_planes = 0;
-
- for (i = 0; i < dm->dc->caps.max_planes; ++i) {
- struct dc_plane_cap *plane = >dc->caps.planes[i];
-
- if (plane->type == DC_PLANE_TYPE_DCN_UNIVERSAL &&
- plane->blends_with_above && plane->blends_with_below &&
- plane->supports_argb)
- overlay_planes += 1;
- }
-
- overlay_planes = min(overlay_planes, 1);
-
/* There is one primary plane per CRTC */
primary_planes = dm->dc->caps.max_streams;
ASSERT(primary_planes <= AMDGPU_MAX_PLANES);
@@ -2073,8 +2058,10 @@ static int amdgpu_dm_initialize_drm_device(struct
amdgpu_device *adev)
* Order is reversed to match iteration order in atomic check.
*/
for (i = (primary_planes - 1); i >= 0; i--) {
+ plane = >dc->caps.planes[i];
+
if (initialize_plane(dm, mode_info, i,
-DRM_PLANE_TYPE_PRIMARY)) {
+DRM_PLANE_TYPE_PRIMARY, plane)) {
DRM_ERROR("KMS: Failed to initialize primary plane\n");
goto fail;
}
@@ -2085,13 +2072,30 @@ static int amdgpu_dm_initialize_drm_device(struct
amdgpu_device *adev)
* These planes have a higher DRM index than the primary planes since
* they should be considered as having a higher z-order.
* Order is reversed to match iteration order in atomic check.
+*
+* Only support DCN for now, and only expose one so we don't encourage
+* userspace to use up all the pipes.
*/
- for (i = (overlay_planes - 1); i >= 0; i--) {
+ for (i = 0; i < dm->dc->caps.max_planes; ++i) {
+ struct dc_plane_cap *plane = >dc->caps.planes[i];
+
[PATCH 08/20] drm/amd/display: remove min reduction for abm 2.2 level 3
From: Josip Pavic
[Why]
Image brightness compensation for solid color full screen images is
expected to be optimal for ABM 2.2 at level 3. The min reduction that is
currently being enforced prevents this from being achieved.
[How]
Remove the min reduction for ABM 2.2 at level 3
Signed-off-by: Josip Pavic
Reviewed-by: Anthony Koo
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/modules/power/power_helpers.c | 4 ++--
1 file changed, 2 insertions(+), 2 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 efd386f..b3810b8 100644
--- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
+++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
@@ -43,10 +43,10 @@ static const unsigned char max_reduction_table[13] = {
/* Possible ABM 2.2 Min Reduction configs from least aggressive to most
aggressive
* 01 2 3 4 5 6 7 8 9 1011 12
- * 100 100 100 100 100 100 100 90.2 85.1 80.0 80.0 75.3 75.3
%
+ * 100 100 100 100 100 100 100 100 100 92.2 83.1 75.3 75.3 %
*/
static const unsigned char min_reduction_table_v_2_2[13] = {
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xe6, 0xd9, 0xcc, 0xcc, 0xc0, 0xc0};
+0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xeb, 0xd4, 0xc0, 0xc0};
/* Possible ABM 2.2 Max Reduction configs from least aggressive to most
aggressive
* 01 2 3 4 5 6 7 8 9 1011 12
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 14/20] drm/amd/display: fix underflow on boot
From: Eric Yang
[Why]
New seamless boot sequence introduced a bug where front end is disabled
without blanking otg.
[How]
Adjust the condition of blanking otg to match seamless boot.
Signed-off-by: Eric Yang
Reviewed-by: Anthony Koo
Acked-by: Leo Li
Acked-by: Tony Cheng
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index afa8648..f659148 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -979,16 +979,14 @@ static void dcn10_init_pipes(struct dc *dc, struct
dc_state *context)
* to non-preferred front end. If pipe_ctx->stream is not NULL,
* we will use the pipe, so don't disable
*/
- if (pipe_ctx->stream != NULL)
+ if (pipe_ctx->stream != NULL && can_apply_seamless_boot)
continue;
- if (tg->funcs->is_tg_enabled(tg))
- tg->funcs->lock(tg);
-
/* Blank controller using driver code instead of
* command table.
*/
if (tg->funcs->is_tg_enabled(tg)) {
+ tg->funcs->lock(tg);
tg->funcs->set_blank(tg, true);
hwss_wait_for_blank_complete(tg);
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 04/20] drm/amd/display: Calculate link bandwidth in a common function
From: Nikola Cornij
[why]
Currently link bandwidth is calculated in two places, using the same
formula. They should be unified into calling one function.
[how]
Replace all implementations of link bandwidth calculation with a call
to a function.
Signed-off-by: Nikola Cornij
Reviewed-by: Nikola Cornij
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 13 +
drivers/gpu/drm/amd/display/dc/core/dc_link.c| 16 +++-
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 24 +---
drivers/gpu/drm/amd/display/dc/dc_link.h | 3 +++
4 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 3dd57140..bce263d 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -590,6 +590,19 @@ void dc_link_set_test_pattern(struct dc_link *link,
cust_pattern_size);
}
+uint32_t dc_link_bandwidth_kbps(
+ const struct dc_link *link,
+ const struct dc_link_settings *link_setting)
+{
+ uint32_t link_bw_kbps = link_setting->link_rate *
LINK_RATE_REF_FREQ_IN_KHZ; /* bytes per sec */
+
+ link_bw_kbps *= 8; /* 8 bits per byte*/
+ link_bw_kbps *= link_setting->lane_count;
+
+ return link_bw_kbps;
+
+}
+
static void destruct(struct dc *dc)
{
dc_release_state(dc->current_state);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index cf5a120..9db5a55 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -58,7 +58,6 @@
**/
enum {
- LINK_RATE_REF_FREQ_IN_MHZ = 27,
PEAK_FACTOR_X1000 = 1006,
/*
* Some receivers fail to train on first try and are good
@@ -2289,14 +2288,13 @@ void core_link_resume(struct dc_link *link)
static struct fixed31_32 get_pbn_per_slot(struct dc_stream_state *stream)
{
- struct dc_link_settings *link_settings =
- >link->cur_link_settings;
- uint32_t link_rate_in_mbps =
- link_settings->link_rate * LINK_RATE_REF_FREQ_IN_MHZ;
- struct fixed31_32 mbps = dc_fixpt_from_int(
- link_rate_in_mbps * link_settings->lane_count);
-
- return dc_fixpt_div_int(mbps, 54);
+ struct fixed31_32 mbytes_per_sec;
+ uint32_t link_rate_in_mbytes_per_sec =
dc_link_bandwidth_kbps(stream->link, >link->cur_link_settings);
+ link_rate_in_mbytes_per_sec /= 8000; /* Kbits to MBytes */
+
+ mbytes_per_sec = dc_fixpt_from_int(link_rate_in_mbytes_per_sec);
+
+ return dc_fixpt_div_int(mbytes_per_sec, 54);
}
static int get_color_depth(enum dc_color_depth color_depth)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 491d13d..0d8ef8f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -1533,22 +1533,6 @@ static bool decide_fallback_link_setting(
return true;
}
-static uint32_t bandwidth_in_kbps_from_link_settings(
- const struct dc_link_settings *link_setting)
-{
- uint32_t link_rate_in_kbps = link_setting->link_rate *
- LINK_RATE_REF_FREQ_IN_KHZ;
-
- uint32_t lane_count = link_setting->lane_count;
- uint32_t kbps = link_rate_in_kbps;
-
- kbps *= lane_count;
- kbps *= 8; /* 8 bits per byte*/
-
- return kbps;
-
-}
-
bool dp_validate_mode_timing(
struct dc_link *link,
const struct dc_crtc_timing *timing)
@@ -1574,7 +1558,7 @@ bool dp_validate_mode_timing(
*/
req_bw = dc_bandwidth_in_kbps_from_timing(timing);
- max_bw = bandwidth_in_kbps_from_link_settings(link_setting);
+ max_bw = dc_link_bandwidth_kbps(link, link_setting);
if (req_bw <= max_bw) {
/* remember the biggest mode here, during
@@ -1609,7 +1593,8 @@ static bool decide_dp_link_settings(struct dc_link *link,
struct dc_link_setting
*/
while (current_link_setting.link_rate <=
link->verified_link_cap.link_rate) {
- link_bw = bandwidth_in_kbps_from_link_settings(
+ link_bw = dc_link_bandwidth_kbps(
+ link,
_link_setting);
if (req_bw <= link_bw) {
*link_setting = current_link_setting;
@@ -1660,7 +1645,8 @@ static bool decide_edp_link_settings(struct dc_link
*link, struct dc_link_settin
*/
while (current_link_setting.link_rate <=
link->verified_link_cap.link_rate) {
- link_bw = bandwidth_in_kbps_from_link_settings(
+ link_bw =
[PATCH 10/20] drm/amd/display: Set surface color space from DRM plane state
From: Nicholas Kazlauskas
[Why]
We need DC's color space to match the color encoding and color space
specified by userspace to correctly render YUV surfaces.
[How]
Convert the DRM color encoding and color range properties to the
appropriate DC colorspace option and update the color space when
performing surface updates.
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Leo Li
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 53 ++-
1 file changed, 52 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 697af51..3588ca7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2817,6 +2817,50 @@ fill_blending_from_plane_state(struct drm_plane_state
*plane_state,
}
}
+static int
+fill_plane_color_attributes(const struct drm_plane_state *plane_state,
+ const struct dc_plane_state *dc_plane_state,
+ enum dc_color_space *color_space)
+{
+ bool full_range;
+
+ *color_space = COLOR_SPACE_SRGB;
+
+ /* DRM color properties only affect non-RGB formats. */
+ if (dc_plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN)
+ return 0;
+
+ full_range = (plane_state->color_range == DRM_COLOR_YCBCR_FULL_RANGE);
+
+ switch (plane_state->color_encoding) {
+ case DRM_COLOR_YCBCR_BT601:
+ if (full_range)
+ *color_space = COLOR_SPACE_YCBCR601;
+ else
+ *color_space = COLOR_SPACE_YCBCR601_LIMITED;
+ break;
+
+ case DRM_COLOR_YCBCR_BT709:
+ if (full_range)
+ *color_space = COLOR_SPACE_YCBCR709;
+ else
+ *color_space = COLOR_SPACE_YCBCR709_LIMITED;
+ break;
+
+ case DRM_COLOR_YCBCR_BT2020:
+ if (full_range)
+ *color_space = COLOR_SPACE_2020_YCBCR;
+ else
+ return -EINVAL;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int fill_plane_attributes(struct amdgpu_device *adev,
struct dc_plane_state *dc_plane_state,
struct drm_plane_state *plane_state,
@@ -2838,6 +2882,11 @@ static int fill_plane_attributes(struct amdgpu_device
*adev,
if (ret)
return ret;
+ ret = fill_plane_color_attributes(plane_state, dc_plane_state,
+ _plane_state->color_space);
+ if (ret)
+ return ret;
+
/*
* Always set input transfer function, since plane state is refreshed
* every time.
@@ -5105,8 +5154,10 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
bundle->scaling_infos[planes_count].clip_rect =
dc_plane->clip_rect;
bundle->surface_updates[planes_count].scaling_info =
>scaling_infos[planes_count];
+ fill_plane_color_attributes(
+ new_plane_state, dc_plane,
+ >plane_infos[planes_count].color_space);
- bundle->plane_infos[planes_count].color_space =
dc_plane->color_space;
bundle->plane_infos[planes_count].format = dc_plane->format;
bundle->plane_infos[planes_count].plane_size =
dc_plane->plane_size;
bundle->plane_infos[planes_count].rotation = dc_plane->rotation;
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 02/20] drm/amd/display: use proper formula to calculate bandwidth from timing
From: Wenjing Liu
[why]
The existing calculation uses a wrong formula to
calculate bandwidth from timing.
[how]
Expose the existing proper function that calculates the bandwidth,
so dc_link can use it to calculate timing bandwidth correctly.
Signed-off-by: Wenjing Liu
Reviewed-by: Jun Lei
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c| 48 +-
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 51 +---
drivers/gpu/drm/amd/display/dc/dc_link.h | 2 +
3 files changed, 51 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index afa8860..2b161dc 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -2321,7 +2321,7 @@ static struct fixed31_32 get_pbn_from_timing(struct
pipe_ctx *pipe_ctx)
uint32_t denominator;
bpc = get_color_depth(pipe_ctx->stream_res.pix_clk_params.color_depth);
- kbps = pipe_ctx->stream_res.pix_clk_params.requested_pix_clk_100hz / 10
* bpc * 3;
+ kbps = dc_bandwidth_in_kbps_from_timing(_ctx->stream->timing);
/*
* margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
@@ -2742,3 +2742,49 @@ void dc_link_enable_hpd_filter(struct dc_link *link,
bool enable)
}
}
+uint32_t dc_bandwidth_in_kbps_from_timing(
+ const struct dc_crtc_timing *timing)
+{
+ uint32_t bits_per_channel = 0;
+ uint32_t kbps;
+
+ switch (timing->display_color_depth) {
+ case COLOR_DEPTH_666:
+ bits_per_channel = 6;
+ break;
+ case COLOR_DEPTH_888:
+ bits_per_channel = 8;
+ break;
+ case COLOR_DEPTH_101010:
+ bits_per_channel = 10;
+ break;
+ case COLOR_DEPTH_121212:
+ bits_per_channel = 12;
+ break;
+ case COLOR_DEPTH_141414:
+ bits_per_channel = 14;
+ break;
+ case COLOR_DEPTH_161616:
+ bits_per_channel = 16;
+ break;
+ default:
+ break;
+ }
+
+ ASSERT(bits_per_channel != 0);
+
+ kbps = timing->pix_clk_100hz / 10;
+ kbps *= bits_per_channel;
+
+ if (timing->flags.Y_ONLY != 1) {
+ /*Only YOnly make reduce bandwidth by 1/3 compares to RGB*/
+ kbps *= 3;
+ if (timing->pixel_encoding == PIXEL_ENCODING_YCBCR420)
+ kbps /= 2;
+ else if (timing->pixel_encoding == PIXEL_ENCODING_YCBCR422)
+ kbps = kbps * 2 / 3;
+ }
+
+ return kbps;
+
+}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 528c962..491d13d 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -1533,53 +1533,6 @@ static bool decide_fallback_link_setting(
return true;
}
-static uint32_t bandwidth_in_kbps_from_timing(
- const struct dc_crtc_timing *timing)
-{
- uint32_t bits_per_channel = 0;
- uint32_t kbps;
-
- switch (timing->display_color_depth) {
- case COLOR_DEPTH_666:
- bits_per_channel = 6;
- break;
- case COLOR_DEPTH_888:
- bits_per_channel = 8;
- break;
- case COLOR_DEPTH_101010:
- bits_per_channel = 10;
- break;
- case COLOR_DEPTH_121212:
- bits_per_channel = 12;
- break;
- case COLOR_DEPTH_141414:
- bits_per_channel = 14;
- break;
- case COLOR_DEPTH_161616:
- bits_per_channel = 16;
- break;
- default:
- break;
- }
-
- ASSERT(bits_per_channel != 0);
-
- kbps = timing->pix_clk_100hz / 10;
- kbps *= bits_per_channel;
-
- if (timing->flags.Y_ONLY != 1) {
- /*Only YOnly make reduce bandwidth by 1/3 compares to RGB*/
- kbps *= 3;
- if (timing->pixel_encoding == PIXEL_ENCODING_YCBCR420)
- kbps /= 2;
- else if (timing->pixel_encoding == PIXEL_ENCODING_YCBCR422)
- kbps = kbps * 2 / 3;
- }
-
- return kbps;
-
-}
-
static uint32_t bandwidth_in_kbps_from_link_settings(
const struct dc_link_settings *link_setting)
{
@@ -1620,7 +1573,7 @@ bool dp_validate_mode_timing(
link_setting = >verified_link_cap;
*/
- req_bw = bandwidth_in_kbps_from_timing(timing);
+ req_bw = dc_bandwidth_in_kbps_from_timing(timing);
max_bw = bandwidth_in_kbps_from_link_settings(link_setting);
if (req_bw <= max_bw) {
@@ -1739,7 +1692,7 @@ void decide_link_settings(struct dc_stream_state *stream,
struct dc_link *link;
[PATCH 01/20] drm/amd/display: fix dp_hdmi_max_pixel_clk units
From: SivapiriyanKumarasamy
[Why]
We are incorrectly using dp_hdmi_max_pixel_clk because the units are not clear.
[How]
Rename to dp_hdmi_max_pixel_clk_in_khz, and change mode timing validation to use
the value correctly.
Signed-off-by: SivapiriyanKumarasamy
Reviewed-by: Aric Cyr
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c| 2 +-
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 6 +++---
drivers/gpu/drm/amd/display/dc/dc_types.h| 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 8720c40..afa8860 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -2150,7 +2150,7 @@ static bool dp_active_dongle_validate_timing(
return false;
}
- if (get_timing_pixel_clock_100hz(timing) >
(dongle_caps->dp_hdmi_max_pixel_clk * 10))
+ if (get_timing_pixel_clock_100hz(timing) >
(dongle_caps->dp_hdmi_max_pixel_clk_in_khz * 10))
return false;
return true;
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 063d019..528c962 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -2304,8 +2304,8 @@ static void get_active_converter_info(
hdmi_caps = {.raw = det_caps[3] };
union dwnstream_port_caps_byte2
hdmi_color_caps = {.raw = det_caps[2] };
-
link->dpcd_caps.dongle_caps.dp_hdmi_max_pixel_clk =
- det_caps[1] * 25000;
+
link->dpcd_caps.dongle_caps.dp_hdmi_max_pixel_clk_in_khz =
+ det_caps[1] * 2500;
link->dpcd_caps.dongle_caps.is_dp_hdmi_s3d_converter =
hdmi_caps.bits.FRAME_SEQ_TO_FRAME_PACK;
@@ -2322,7 +2322,7 @@ static void get_active_converter_info(
translate_dpcd_max_bpc(
hdmi_color_caps.bits.MAX_BITS_PER_COLOR_COMPONENT);
- if
(link->dpcd_caps.dongle_caps.dp_hdmi_max_pixel_clk != 0)
+ if
(link->dpcd_caps.dongle_caps.dp_hdmi_max_pixel_clk_in_khz != 0)
link->dpcd_caps.dongle_caps.extendedCapValid = true;
}
diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h
b/drivers/gpu/drm/amd/display/dc/dc_types.h
index 130d039..7402f5a 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_types.h
@@ -404,7 +404,7 @@ struct dc_dongle_caps {
bool is_dp_hdmi_ycbcr422_converter;
bool is_dp_hdmi_ycbcr420_converter;
uint32_t dp_hdmi_max_bpc;
- uint32_t dp_hdmi_max_pixel_clk;
+ uint32_t dp_hdmi_max_pixel_clk_in_khz;
};
/* Scaling format */
enum scaling_transformation {
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 12/20] drm/amd/display: init dc_config before rest of DC init
From: Anthony Koo
[Why]
In some cases we want DC init to take in some config options
[How]
Init dc_config before rest of DC init
Signed-off-by: Anthony Koo
Reviewed-by: Aric Cyr
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 1ebfa6c..ceeacd7 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -660,6 +660,8 @@ static bool construct(struct dc *dc,
#endif
enum dce_version dc_version = DCE_VERSION_UNKNOWN;
+ dc->config = init_params->flags;
+
memcpy(>bb_overrides, _params->bb_overrides,
sizeof(dc->bb_overrides));
dc_dceip = kzalloc(sizeof(*dc_dceip), GFP_KERNEL);
@@ -854,8 +856,6 @@ struct dc *dc_create(const struct dc_init_data *init_params)
if (dc->res_pool->dmcu != NULL)
dc->versions.dmcu_version = dc->res_pool->dmcu->dmcu_version;
- dc->config = init_params->flags;
-
dc->build_id = DC_BUILD_ID;
DC_LOG_DC("Display Core initialized\n");
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 09/20] drm/amd/display: prefer preferred link cap over verified link settings
From: Wenjing Liu
[why]
when preferred link cap is set, we should always use
preferred in all validation.
we should not use preferred for some validation but use
verified for others.
[how]
create getter function that gets verified link cap.
if preferred is set, return preferred link settings instead.
Signed-off-by: Wenjing Liu
Reviewed-by: Nikola Cornij
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 9 +
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
drivers/gpu/drm/amd/display/dc/dc_link.h | 3 +++
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index bce263d..1ebfa6c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -603,6 +603,15 @@ uint32_t dc_link_bandwidth_kbps(
}
+const struct dc_link_settings *dc_link_get_verified_link_cap(
+ const struct dc_link *link)
+{
+ if (link->preferred_link_setting.lane_count != LANE_COUNT_UNKNOWN &&
+ link->preferred_link_setting.link_rate !=
LINK_RATE_UNKNOWN)
+ return >preferred_link_setting;
+ return >verified_link_cap;
+}
+
static void destruct(struct dc *dc)
{
dc_release_state(dc->current_state);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 0d8ef8f..acb4f82 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -1549,7 +1549,7 @@ bool dp_validate_mode_timing(
return true;
/* We always use verified link settings */
- link_setting = >verified_link_cap;
+ link_setting = dc_link_get_verified_link_cap(link);
/* TODO: DYNAMIC_VALIDATION needs to be implemented */
/*if (flags.DYNAMIC_VALIDATION == 1 &&
diff --git a/drivers/gpu/drm/amd/display/dc/dc_link.h
b/drivers/gpu/drm/amd/display/dc/dc_link.h
index 7b61fb7..4e26d6e 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_link.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_link.h
@@ -250,6 +250,9 @@ uint32_t dc_link_bandwidth_kbps(
const struct dc_link *link,
const struct dc_link_settings *link_setting);
+const struct dc_link_settings *dc_link_get_verified_link_cap(
+ const struct dc_link *link);
+
bool dc_submit_i2c(
struct dc *dc,
uint32_t link_index,
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 11/20] drm/amd/display: Call hwss.set_cursor_sdr_white_level, if available
From: SivapiriyanKumarasamy
[Why]
In HDR configurations, the cursor - in SDR - needs to have it's white
level boosted.
[How]
Program the cursor boost in update_dchubp_dpp like the other cursor
attributes.
Signed-off-by: SivapiriyanKumarasamy
Reviewed-by: Anthony Koo
Acked-by: Leo Li
Acked-by: Reza Amini
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index 976812a..afa8648 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -2138,6 +2138,9 @@ void update_dchubp_dpp(
if (pipe_ctx->stream->cursor_attributes.address.quad_part != 0) {
dc->hwss.set_cursor_position(pipe_ctx);
dc->hwss.set_cursor_attribute(pipe_ctx);
+
+ if (dc->hwss.set_cursor_sdr_white_level)
+ dc->hwss.set_cursor_sdr_white_level(pipe_ctx);
}
if (plane_state->update_flags.bits.full_update) {
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 06/20] drm/amd/display: return correct dc_status for dcn10_validate_global
From: Su Sung Chung
[Why]
Before it was returning false in the case of failure even though return type
should be enum dc_status
[How]
Return DC_FAIL_UNSUPPORTED_1 instead
Signed-off-by: Su Sung Chung
Reviewed-by: Eric Yang
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 2 +-
drivers/gpu/drm/amd/display/dc/inc/core_status.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index 7c37836..79f4fbb 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -1146,7 +1146,7 @@ static enum dc_status dcn10_validate_global(struct dc
*dc, struct dc_state *cont
continue;
if (context->stream_status[i].plane_count > 2)
- return false;
+ return DC_FAIL_UNSUPPORTED_1;
for (j = 0; j < context->stream_status[i].plane_count; j++) {
struct dc_plane_state *plane =
diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_status.h
b/drivers/gpu/drm/amd/display/dc/inc/core_status.h
index 2e61a22..8dca3b7 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/core_status.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/core_status.h
@@ -43,7 +43,7 @@ enum dc_status {
DC_FAIL_BANDWIDTH_VALIDATE = 13, /* BW and Watermark validation */
DC_FAIL_SCALING = 14,
DC_FAIL_DP_LINK_TRAINING = 15,
-
+ DC_FAIL_UNSUPPORTED_1 = 18,
DC_ERROR_UNEXPECTED = -1
};
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 05/20] drm/amd/display: Use plane->color_space for dpp if specified
From: Nicholas Kazlauskas [Why] The input color space for the plane was previously ignored even if it was set. If a limited range YUV format was given to DC then the wrong color transformation matrix was being used since DC assumed that it was full range instead. [How] Respect the given color_space format for the plane if it isn't COLOR_SPACE_UNKNOWN. Otherwise, use the implicit default since DM didn't specify. Signed-off-by: Nicholas Kazlauskas Reviewed-by: Sun peng Li Acked-by: Aric Cyr Acked-by: Leo Li --- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c | 6 +- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c index f91e4b4..6f4b247 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c @@ -385,6 +385,10 @@ void dpp1_cnv_setup ( default: break; } + + /* Set default color space based on format if none is given. */ + color_space = input_color_space ? input_color_space : color_space; + REG_SET(CNVC_SURFACE_PIXEL_FORMAT, 0, CNVC_SURFACE_PIXEL_FORMAT, pixel_format); REG_UPDATE(FORMAT_CONTROL, FORMAT_CONTROL__ALPHA_EN, alpha_en); @@ -396,7 +400,7 @@ void dpp1_cnv_setup ( for (i = 0; i < 12; i++) tbl_entry.regval[i] = input_csc_color_matrix.matrix[i]; - tbl_entry.color_space = input_color_space; + tbl_entry.color_space = color_space; if (color_space >= COLOR_SPACE_YCBCR601) select = INPUT_CSC_SELECT_ICSC; diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c index 4969fa5..976812a 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c @@ -1949,7 +1949,7 @@ static void update_dpp(struct dpp *dpp, struct dc_plane_state *plane_state) plane_state->format, EXPANSION_MODE_ZERO, plane_state->input_csc_color_matrix, - COLOR_SPACE_YCBCR601_LIMITED); + plane_state->color_space); //set scale and bias registers build_prescale_params(_params, plane_state); -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 07/20] drm/amd/display: 3.2.25
From: Aric Cyr Signed-off-by: Aric Cyr Reviewed-by: Aric Cyr Acked-by: Leo Li --- drivers/gpu/drm/amd/display/dc/dc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 54b0759..4cb9269 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -42,7 +42,7 @@ #include "inc/hw/dmcu.h" #include "dml/display_mode_lib.h" -#define DC_VER "3.2.24" +#define DC_VER "3.2.25" #define MAX_SURFACES 3 #define MAX_PLANES 6 -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 03/20] drm/amd/display: fix clk_mgr naming
From: Dmytro Laktyushkin
clk_mgr is called dccg in dc_state, this change fixes that
Signed-off-by: Dmytro Laktyushkin
Reviewed-by: Eric Bernstein
Acked-by: Leo Li
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 2 +-
drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +-
drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 4 ++--
drivers/gpu/drm/amd/display/dc/inc/core_types.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 2b161dc..cf5a120 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1399,7 +1399,7 @@ static enum dc_status enable_link_dp(
pipe_ctx->stream_res.pix_clk_params.requested_sym_clk =
link_settings.link_rate * LINK_RATE_REF_FREQ_IN_KHZ;
- state->dccg->funcs->update_clocks(state->dccg, state, false);
+ state->clk_mgr->funcs->update_clocks(state->clk_mgr, state, false);
dp_enable_link_phy(
link,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index d0ed95e..f798fc2 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2064,7 +2064,7 @@ void dc_resource_state_construct(
const struct dc *dc,
struct dc_state *dst_ctx)
{
- dst_ctx->dccg = dc->res_pool->clk_mgr;
+ dst_ctx->clk_mgr = dc->res_pool->clk_mgr;
}
/**
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index a1c4d26c..7ac50ab 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1166,8 +1166,8 @@ static void build_audio_output(
if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
pipe_ctx->stream->signal ==
SIGNAL_TYPE_DISPLAY_PORT_MST) {
audio_output->pll_info.dp_dto_source_clock_in_khz =
- state->dccg->funcs->get_dp_ref_clk_frequency(
- state->dccg);
+ state->clk_mgr->funcs->get_dp_ref_clk_frequency(
+ state->clk_mgr);
}
audio_output->pll_info.feed_back_divider =
diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h
b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
index c3f820c..827541e 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
@@ -301,7 +301,7 @@ struct dc_state {
struct dcn_bw_internal_vars dcn_bw_vars;
#endif
- struct clk_mgr *dccg;
+ struct clk_mgr *clk_mgr;
struct {
bool full_update_needed : 1;
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 00/20] DC Patches Apr 03, 2019
From: Leo Li Summary of changed * Add DM support for YUV planes * Fix potential S3 resume bug Anthony Koo (2): drm/amd/display: init dc_config before rest of DC init drm/amd/display: disable link before changing link settings Aric Cyr (1): drm/amd/display: 3.2.25 Dmytro Laktyushkin (2): drm/amd/display: fix clk_mgr naming drm/amd/display: fix is odm head pipe logic Eric Yang (1): drm/amd/display: fix underflow on boot Josip Pavic (1): drm/amd/display: remove min reduction for abm 2.2 level 3 Leo Li (2): drm/amd/display: Recreate private_obj->state during S3 resume drm/amd/display: Clean up locking in dcn*_apply_ctx_for_surface() Martin Leung (1): drm/amd/display: extending AUX SW Timeout Murton Liu (1): drm/amd/display: HDR visual confirmation incorrectly reports black color Nicholas Kazlauskas (3): drm/amd/display: Use plane->color_space for dpp if specified drm/amd/display: Set surface color space from DRM plane state drm/amd/display: Pass plane caps into amdgpu_dm_plane_init Nikola Cornij (1): drm/amd/display: Calculate link bandwidth in a common function SivapiriyanKumarasamy (2): drm/amd/display: fix dp_hdmi_max_pixel_clk units drm/amd/display: Call hwss.set_cursor_sdr_white_level, if available Su Sung Chung (1): drm/amd/display: return correct dc_status for dcn10_validate_global Wenjing Liu (2): drm/amd/display: use proper formula to calculate bandwidth from timing drm/amd/display: prefer preferred link cap over verified link settings drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 123 - drivers/gpu/drm/amd/display/dc/core/dc.c | 26 - drivers/gpu/drm/amd/display/dc/core/dc_link.c | 77 +++-- drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 83 ++ drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 13 +-- drivers/gpu/drm/amd/display/dc/dc.h| 2 +- drivers/gpu/drm/amd/display/dc/dc_link.h | 8 ++ drivers/gpu/drm/amd/display/dc/dc_types.h | 2 +- drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 9 +- drivers/gpu/drm/amd/display/dc/dce/dce_aux.h | 6 +- .../amd/display/dc/dce110/dce110_hw_sequencer.c| 4 +- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c | 6 +- .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 79 - .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h | 4 + .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 2 +- drivers/gpu/drm/amd/display/dc/inc/core_status.h | 2 +- drivers/gpu/drm/amd/display/dc/inc/core_types.h| 2 +- .../drm/amd/display/modules/power/power_helpers.c | 4 +- 18 files changed, 284 insertions(+), 168 deletions(-) -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/8] drm/amdgpu: fix ATC handling for Ryzen
On 2019-04-03 1:24 p.m., Koenig, Christian wrote:
> Am 01.04.19 um 20:58 schrieb Kuehling, Felix:
>> On 2019-04-01 2:03 p.m., Christian König wrote:
>>> Am 01.04.19 um 19:59 schrieb Kuehling, Felix:
On 2019-04-01 7:23 a.m., Christian König wrote:
> Am 30.03.19 um 01:41 schrieb Kuehling, Felix:
>> Patches 1-3 are Reviewed-by: Felix Kuehling
> Thanks.
>
>> About the direct mode, that removes a bunch of synchronization, so it
>> must make some assumptions about the state of the page tables. What
>> makes that safe?
> Direct mode is only supposed to be used during page fault handling.
>
> E.g. we know that the page tables are in the correct place in this
> situation because the hardware is hammering on a PTE and waiting for
> it to become valid.
A fence could also indicate a concurrent modification of the page table.
For example a PTB may be allocated and initialized concurrently, not in
direct mode. Would direct mode need to wait for a fence that indicates
completion of the PTB initialization? Or do we have some way to ensure
such concurrent allocation and initialization of a PTB cannot happen?
>>> Yeah, that is a very good question I haven't solved yet either.
>>>
>>> My currently best idea is to separate the address space, e.g. use the
>>> lower address space for on demand paging and the higher with classic
>>> pre-filled page tables for the MM and display engines.
>> That may work for graphics, but doesn't work for KFD. I need the ability
>> to mix pre-filled page tables with HMM in the same SVM address space.
> Even after thinking for multiple days about it I can't of hand find a
> way to make this work.
>
>> That's why I was thinking that all page table updates for a given VM
>> would need to use the same method.
> Well what exactly do you mean with that? Essentially there are two methods:
>
> 1. Pre-fill the page tables before accessing them with the hardware.
>
> 2. Fill on demand with page faults.
>
> I don't think we can mix those two methods together in the same address
> range.
That's what I was hoping to do. For example an application could use
"old" BO-based memory management APIs that pre-fill page tables with
"new" HMM-based memory management APIs that rely on page faults. Those
may be different libraries written in different languages running in the
same application. E.g. a GPU BLAS implementation that's optimized and
uses old-style memory allocations linked to an OpenMP application that
relies on HMM.
If that's not possible, I'd need to emulate all the old memory APIs on
top of HMM. I was hoping to avoid that.
Even when page faults are enabled, we want to be able to pre-fault stuff
to avoid the performance it on the first access. Are you saying that
won't be possible?
Regards,
Felix
>
> E.g. we can say to use pre-fill for MM engines in the upper range and on
> demand filling in the lower range, but we can't mix them.
>
> Regards,
> Christian.
>
>> Regards,
>> Felix
>>
>>> Christian.
>>>
Regards,
Felix
> Christian.
>
>> Is it safe to use direct-mode on a
>> per-page-table-update basis? Or do all page table updates have to go
>> through direct mode to avoid hazards? If yes, then maybe this
>> should be
>> a property of the VM rather than a parameter that gets passed to a
>> bunch
>> of function calls.
>>
>> Regards,
>> Felix
>>
>> On 2019-03-29 6:45 a.m., Christian König wrote:
>>> Otherwise we don't correctly use translate further.
>>>
>>> Signed-off-by: Christian König
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 13 -
>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 3d221f044183..059d9802e713 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -767,14 +767,17 @@ static int amdgpu_vm_clear_bo(struct
>>> amdgpu_device *adev,
>>> addr = 0;
>>> if (ats_entries) {
>>> - uint64_t ats_value;
>>> + uint64_t value = 0, flags;
>>> - ats_value = AMDGPU_PTE_DEFAULT_ATC;
>>> - if (level != AMDGPU_VM_PTB)
>>> - ats_value |= AMDGPU_PDE_PTE;
>>> + flags = AMDGPU_PTE_DEFAULT_ATC;
>>> + if (level != AMDGPU_VM_PTB) {
>>> + /* Handle leaf PDEs as PTEs */
>>> + flags |= AMDGPU_PDE_PTE;
>>> + amdgpu_gmc_get_vm_pde(adev, level, , );
>>> + }
>>> r = vm->update_funcs->update(, bo, addr, 0,
>>> ats_entries,
>>> - 0, ats_value);
>>> + value, flags);
>>> if (r)
>>> return r;
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Wed, Apr 03, 2019 at 09:20:39AM -0700, Paul E. McKenney wrote:
> On Wed, Apr 03, 2019 at 10:27:42AM -0400, Mathieu Desnoyers wrote:
> > - On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
> >
> > > On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> > >> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
> > >>
> > >> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> > >> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com
> > >> >> wrote:
> > >> >>
> > >> >> > Hello!
> > >> >> >
> > >> >> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> > >> >> > by loadable modules. The reason for this prohibition is the fact
> > >> >> > that using these two macros within modules requires that the size of
> > >> >> > the reserved region be increased, which is not something we want to
> > >> >> > be doing all that often. Instead, loadable modules should define an
> > >> >> > srcu_struct and invoke init_srcu_struct() from their module_init
> > >> >> > function
> > >> >> > and cleanup_srcu_struct() from their module_exit function. Note
> > >> >> > that
> > >> >> > modules using call_srcu() will also need to invoke srcu_barrier()
> > >> >> > from
> > >> >> > their module_exit function.
> > >> >>
> > >> >> This arbitrary API limitation seems weird.
> > >> >>
> > >> >> Isn't there a way to allow modules to use DEFINE_SRCU and
> > >> >> DEFINE_STATIC_SRCU
> > >> >> while implementing them with dynamic allocation under the hood ?
> > >> >
> > >> > Although call_srcu() already has initialization hooks, some would
> > >> > also be required in srcu_read_lock(), and I am concerned about adding
> > >> > memory allocation at that point, especially given the possibility
> > >> > of memory-allocation failure. And the possibility that the first
> > >> > srcu_read_lock() happens in an interrupt handler or similar.
> > >> >
> > >> > Or am I missing a trick here?
> > >>
> > >> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> > >> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> > >> would additionally lookup that section on module load, and deal with
> > >> those statically defined SRCU entries as if they were dynamically
> > >> allocated ones. It would of course cleanup those resources on module
> > >> unload.
> > >>
> > >> Am I missing some subtlety there ?
> > >
> > > If I understand you correctly, that is actually what is already done. The
> > > size of this dedicated section is currently set by PERCPU_MODULE_RESERVE,
> > > and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring that
> > > this to be increased frequently. That led to a request that something
> > > be done, in turn leading to this patch series.
> >
> > I think we are not expressing quite the same idea.
> >
> > AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data within
> > modules,
> > which ends up using percpu module reserved memory.
> >
> > My idea is to make DEFINE*_SRCU have a different behavior under #ifdef
> > MODULE.
> > It could emit a _global variable_ (_not_ per-cpu) within a new section. That
> > section would then be used by module init/exit code to figure out what "srcu
> > descriptors" are present in the modules. It would therefore rely on dynamic
> > allocation for those, therefore removing the need to involve the percpu
> > module
> > reserved pool at all.
> >
> > >
> > > I don't see a way around this short of changing module loading to do
> > > alloc_percpu() and then updating the relocation based on this result.
> > > Which would admittedly be far more convenient. I was assuming that
> > > this would be difficult due to varying CPU offsets or the like.
> > >
> > > But if it can be done reasonably, it would be quite a bit nicer than
> > > forcing dynamic allocation in cases where it is not otherwise needed.
> >
> > Hopefully my explanation above helps clear out what I have in mind.
> >
> > You can find similar tricks performed by include/linux/tracepoint.h:
> >
> > #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> > static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> > {
> > return offset_to_ptr(p);
> > }
> >
> > #define __TRACEPOINT_ENTRY(name)\
> > asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
> > " .balign 4 \n" \
> > " .long __tracepoint_" #name " - . \n" \
> > " .previous \n")
> > #else
> > static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> > {
> > return *p;
> > }
> >
> > #define __TRACEPOINT_ENTRY(name) \
> > static tracepoint_ptr_t __tracepoint_ptr_##name __used \
> >
Re: How to get useful information other than "the whole system locks up"?
On Wed, Apr 3, 2019 at 2:58 AM Braiam wrote: > > Hi, > > I have a Sapphire Technology Hawaii XT (R9 290X) using amdgpu driver > with kernel 5.1.0-rc3. > The issue happens with current 4.19.0 debian testing, 4.20-trunk, > 5.0.0-trunk and rc2 and 3. > > It usually happens when I'm reproducing video, but I haven't figured > out a way to reproduce it. It > happened once without reproducing. I'm aware that the support is > experimental, but radeon > driver doesn't seems capable of direct rendering on this card dropping > to llvmepipe. Radeon should work out of the box. Maybe something is messed up with your install? > > I had a ssh server installed in case I could log in while it crashes, > and the only relevant > line I found was: > > drm:amdgpu job timeout [amdgpu]] **ERROR** ring gfx timeout, signaled > seq=399919, emitted seq=399921 > > But that turned several bug reports which seems to have been fixed and > the context and symptoms are too different to mine. > You appear to be experiencing a GPU lockup. Unfortunately, there can be many things that cause it, so it really helps to have a good reproducer case. You might try a newer version of mesa or llvm. What does your "reproducing video" work flow use? What apps, APIs are involved? Alex > I have tried forcing the amdgpu xorg driver with same results (was > using radeon). > > -- > Braiam > ___ > 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/amdkfd: Add picasso pci id
Reviewed-by: Kent Russell
Kent
-Original Message-
From: amd-gfx On Behalf Of Alex Deucher
Sent: Wednesday, April 3, 2019 1:35 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: [PATCH] drm/amdkfd: Add picasso pci id
Picasso is a new raven variant.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdkfd/kfd_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index b3cdbf79f47b..2fee3063a0d6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -320,6 +320,7 @@ static const struct kfd_deviceid supported_devices[] = {
{ 0x9876, _device_info }, /* Carrizo */
{ 0x9877, _device_info }, /* Carrizo */
{ 0x15DD, _device_info }, /* Raven */
+ { 0x15D8, _device_info }, /* Raven */
#endif
{ 0x67A0, _device_info },/* Hawaii */
{ 0x67A1, _device_info },/* Hawaii */
--
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
[PATCH] drm/amdkfd: Add picasso pci id
Picasso is a new raven variant.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdkfd/kfd_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index b3cdbf79f47b..2fee3063a0d6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -320,6 +320,7 @@ static const struct kfd_deviceid supported_devices[] = {
{ 0x9876, _device_info }, /* Carrizo */
{ 0x9877, _device_info }, /* Carrizo */
{ 0x15DD, _device_info }, /* Raven */
+ { 0x15D8, _device_info }, /* Raven */
#endif
{ 0x67A0, _device_info },/* Hawaii */
{ 0x67A1, _device_info },/* Hawaii */
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/8] drm/amdgpu: fix ATC handling for Ryzen
Am 01.04.19 um 20:58 schrieb Kuehling, Felix:
> On 2019-04-01 2:03 p.m., Christian König wrote:
>> Am 01.04.19 um 19:59 schrieb Kuehling, Felix:
>>> On 2019-04-01 7:23 a.m., Christian König wrote:
Am 30.03.19 um 01:41 schrieb Kuehling, Felix:
> Patches 1-3 are Reviewed-by: Felix Kuehling
Thanks.
> About the direct mode, that removes a bunch of synchronization, so it
> must make some assumptions about the state of the page tables. What
> makes that safe?
Direct mode is only supposed to be used during page fault handling.
E.g. we know that the page tables are in the correct place in this
situation because the hardware is hammering on a PTE and waiting for
it to become valid.
>>> A fence could also indicate a concurrent modification of the page table.
>>> For example a PTB may be allocated and initialized concurrently, not in
>>> direct mode. Would direct mode need to wait for a fence that indicates
>>> completion of the PTB initialization? Or do we have some way to ensure
>>> such concurrent allocation and initialization of a PTB cannot happen?
>> Yeah, that is a very good question I haven't solved yet either.
>>
>> My currently best idea is to separate the address space, e.g. use the
>> lower address space for on demand paging and the higher with classic
>> pre-filled page tables for the MM and display engines.
> That may work for graphics, but doesn't work for KFD. I need the ability
> to mix pre-filled page tables with HMM in the same SVM address space.
Even after thinking for multiple days about it I can't of hand find a
way to make this work.
> That's why I was thinking that all page table updates for a given VM
> would need to use the same method.
Well what exactly do you mean with that? Essentially there are two methods:
1. Pre-fill the page tables before accessing them with the hardware.
2. Fill on demand with page faults.
I don't think we can mix those two methods together in the same address
range.
E.g. we can say to use pre-fill for MM engines in the upper range and on
demand filling in the lower range, but we can't mix them.
Regards,
Christian.
>
> Regards,
> Felix
>
>> Christian.
>>
>>> Regards,
>>> Felix
>>>
>>>
Christian.
> Is it safe to use direct-mode on a
> per-page-table-update basis? Or do all page table updates have to go
> through direct mode to avoid hazards? If yes, then maybe this
> should be
> a property of the VM rather than a parameter that gets passed to a
> bunch
> of function calls.
>
> Regards,
> Felix
>
> On 2019-03-29 6:45 a.m., Christian König wrote:
>> Otherwise we don't correctly use translate further.
>>
>> Signed-off-by: Christian König
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 13 -
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 3d221f044183..059d9802e713 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -767,14 +767,17 @@ static int amdgpu_vm_clear_bo(struct
>> amdgpu_device *adev,
>> addr = 0;
>> if (ats_entries) {
>> - uint64_t ats_value;
>> + uint64_t value = 0, flags;
>> - ats_value = AMDGPU_PTE_DEFAULT_ATC;
>> - if (level != AMDGPU_VM_PTB)
>> - ats_value |= AMDGPU_PDE_PTE;
>> + flags = AMDGPU_PTE_DEFAULT_ATC;
>> + if (level != AMDGPU_VM_PTB) {
>> + /* Handle leaf PDEs as PTEs */
>> + flags |= AMDGPU_PDE_PTE;
>> + amdgpu_gmc_get_vm_pde(adev, level, , );
>> + }
>> r = vm->update_funcs->update(, bo, addr, 0,
>> ats_entries,
>> - 0, ats_value);
>> + value, flags);
>> if (r)
>> return r;
>>> ___
>>> 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: Adjust IB test timeout for XGMI configuration
Thanks , changed as suggested and pushed
Shaoyun.liu
On 2019-04-03 1:12 p.m., Christian König wrote:
> Am 03.04.19 um 17:42 schrieb Liu, Shaoyun:
>> On XGMI configuration the ib test may tooks longer to finish
>>
>> Change-Id: If3afd8eac3c342d32c387804b51fc4a4bdd35d35
>> Signed-off-by: shaoyunl
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 0b8ef2d..6c508d7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -35,6 +35,7 @@
>> #include "amdgpu_trace.h"
>> #define AMDGPU_IB_TEST_TIMEOUT msecs_to_jiffies(1000)
>> +#define AMDGPU_IB_TEST_GFX_XGMI_TIMEOU msecs_to_jiffies(2000)
>> /*
>> * IB
>> @@ -344,7 +345,8 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
>> * cost waiting for it coming back under RUNTIME only
>> */
>> tmo_gfx = 8 * AMDGPU_IB_TEST_TIMEOUT;
>> - }
>> + } else if (adev->gmc.xgmi.hive_id)
>> + tmo_gfx = AMDGPU_IB_TEST_GFX_XGMI_TIMEOU;
>
> A style nit pick here: The "else" branch should have { } as well when
> the "if" has them.
>
> Apart from that the patch is Reviewed-by: Christian König
> .
>
> Christian.
>
>> for (i = 0; i < adev->num_rings; ++i) {
>> struct amdgpu_ring *ring = adev->rings[i];
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Adjust IB test timeout for XGMI configuration
Am 03.04.19 um 17:42 schrieb Liu, Shaoyun:
On XGMI configuration the ib test may tooks longer to finish
Change-Id: If3afd8eac3c342d32c387804b51fc4a4bdd35d35
Signed-off-by: shaoyunl
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 0b8ef2d..6c508d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -35,6 +35,7 @@
#include "amdgpu_trace.h"
#define AMDGPU_IB_TEST_TIMEOUT msecs_to_jiffies(1000)
+#define AMDGPU_IB_TEST_GFX_XGMI_TIMEOU msecs_to_jiffies(2000)
/*
* IB
@@ -344,7 +345,8 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
* cost waiting for it coming back under RUNTIME only
*/
tmo_gfx = 8 * AMDGPU_IB_TEST_TIMEOUT;
- }
+ } else if (adev->gmc.xgmi.hive_id)
+ tmo_gfx = AMDGPU_IB_TEST_GFX_XGMI_TIMEOU;
A style nit pick here: The "else" branch should have { } as well when
the "if" has them.
Apart from that the patch is Reviewed-by: Christian König
.
Christian.
for (i = 0; i < adev->num_rings; ++i) {
struct amdgpu_ring *ring = adev->rings[i];
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Wed, Apr 03, 2019 at 10:27:42AM -0400, Mathieu Desnoyers wrote:
> - On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
>
> > On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> >> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
> >>
> >> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> >> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote:
> >> >>
> >> >> > Hello!
> >> >> >
> >> >> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> >> >> > by loadable modules. The reason for this prohibition is the fact
> >> >> > that using these two macros within modules requires that the size of
> >> >> > the reserved region be increased, which is not something we want to
> >> >> > be doing all that often. Instead, loadable modules should define an
> >> >> > srcu_struct and invoke init_srcu_struct() from their module_init
> >> >> > function
> >> >> > and cleanup_srcu_struct() from their module_exit function. Note that
> >> >> > modules using call_srcu() will also need to invoke srcu_barrier() from
> >> >> > their module_exit function.
> >> >>
> >> >> This arbitrary API limitation seems weird.
> >> >>
> >> >> Isn't there a way to allow modules to use DEFINE_SRCU and
> >> >> DEFINE_STATIC_SRCU
> >> >> while implementing them with dynamic allocation under the hood ?
> >> >
> >> > Although call_srcu() already has initialization hooks, some would
> >> > also be required in srcu_read_lock(), and I am concerned about adding
> >> > memory allocation at that point, especially given the possibility
> >> > of memory-allocation failure. And the possibility that the first
> >> > srcu_read_lock() happens in an interrupt handler or similar.
> >> >
> >> > Or am I missing a trick here?
> >>
> >> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> >> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> >> would additionally lookup that section on module load, and deal with
> >> those statically defined SRCU entries as if they were dynamically
> >> allocated ones. It would of course cleanup those resources on module
> >> unload.
> >>
> >> Am I missing some subtlety there ?
> >
> > If I understand you correctly, that is actually what is already done. The
> > size of this dedicated section is currently set by PERCPU_MODULE_RESERVE,
> > and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring that
> > this to be increased frequently. That led to a request that something
> > be done, in turn leading to this patch series.
>
> I think we are not expressing quite the same idea.
>
> AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data within
> modules,
> which ends up using percpu module reserved memory.
>
> My idea is to make DEFINE*_SRCU have a different behavior under #ifdef MODULE.
> It could emit a _global variable_ (_not_ per-cpu) within a new section. That
> section would then be used by module init/exit code to figure out what "srcu
> descriptors" are present in the modules. It would therefore rely on dynamic
> allocation for those, therefore removing the need to involve the percpu module
> reserved pool at all.
>
> >
> > I don't see a way around this short of changing module loading to do
> > alloc_percpu() and then updating the relocation based on this result.
> > Which would admittedly be far more convenient. I was assuming that
> > this would be difficult due to varying CPU offsets or the like.
> >
> > But if it can be done reasonably, it would be quite a bit nicer than
> > forcing dynamic allocation in cases where it is not otherwise needed.
>
> Hopefully my explanation above helps clear out what I have in mind.
>
> You can find similar tricks performed by include/linux/tracepoint.h:
>
> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> {
> return offset_to_ptr(p);
> }
>
> #define __TRACEPOINT_ENTRY(name)\
> asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
> " .balign 4 \n" \
> " .long __tracepoint_" #name " - . \n" \
> " .previous \n")
> #else
> static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> {
> return *p;
> }
>
> #define __TRACEPOINT_ENTRY(name) \
> static tracepoint_ptr_t __tracepoint_ptr_##name __used \
> __attribute__((section("__tracepoints_ptrs"))) = \
> &__tracepoint_##name
> #endif
>
> [...]
>
> #define DEFINE_TRACE_FN(name, reg, unreg)\
> static const char __tpstrtab_##name[]\
>
[PATCH] drm/amdgpu: Adjust IB test timeout for XGMI configuration
On XGMI configuration the ib test may take longer to finish
Change-Id: If3afd8eac3c342d32c387804b51fc4a4bdd35d35
Signed-off-by: shaoyunl
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 0b8ef2d..5049845 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -35,6 +35,7 @@
#include "amdgpu_trace.h"
#define AMDGPU_IB_TEST_TIMEOUT msecs_to_jiffies(1000)
+#define AMDGPU_IB_TEST_GFX_XGMI_TIMEOUTmsecs_to_jiffies(2000)
/*
* IB
@@ -344,7 +345,8 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
* cost waiting for it coming back under RUNTIME only
*/
tmo_gfx = 8 * AMDGPU_IB_TEST_TIMEOUT;
- }
+ } else if (adev->gmc.xgmi.hive_id)
+ tmo_gfx = AMDGPU_IB_TEST_GFX_XGMI_TIMEOUT;
for (i = 0; i < adev->num_rings; ++i) {
struct amdgpu_ring *ring = adev->rings[i];
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Adjust IB test timeout for XGMI configuration
On Wed, Apr 3, 2019 at 11:42 AM Liu, Shaoyun wrote:
>
> On XGMI configuration the ib test may tooks longer to finish
>
> Change-Id: If3afd8eac3c342d32c387804b51fc4a4bdd35d35
> Signed-off-by: shaoyunl
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 0b8ef2d..6c508d7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -35,6 +35,7 @@
> #include "amdgpu_trace.h"
>
> #define AMDGPU_IB_TEST_TIMEOUT msecs_to_jiffies(1000)
> +#define AMDGPU_IB_TEST_GFX_XGMI_TIMEOU msecs_to_jiffies(2000)
Typo:
TIMEOU -> TIMEOUT
>
> /*
> * IB
> @@ -344,7 +345,8 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
> * cost waiting for it coming back under RUNTIME only
> */
> tmo_gfx = 8 * AMDGPU_IB_TEST_TIMEOUT;
> - }
> + } else if (adev->gmc.xgmi.hive_id)
> + tmo_gfx = AMDGPU_IB_TEST_GFX_XGMI_TIMEOU;
>
Coding style, if any clause has parens, all should. With these issues
fixed, patch is:
Reviewed-by: Alex Deucher
> for (i = 0; i < adev->num_rings; ++i) {
> struct amdgpu_ring *ring = adev->rings[i];
> --
> 2.7.4
>
> ___
> 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
[PATCH] drm/amdgpu: Adjust IB test timeout for XGMI configuration
On XGMI configuration the ib test may tooks longer to finish
Change-Id: If3afd8eac3c342d32c387804b51fc4a4bdd35d35
Signed-off-by: shaoyunl
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 0b8ef2d..6c508d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -35,6 +35,7 @@
#include "amdgpu_trace.h"
#define AMDGPU_IB_TEST_TIMEOUT msecs_to_jiffies(1000)
+#define AMDGPU_IB_TEST_GFX_XGMI_TIMEOU msecs_to_jiffies(2000)
/*
* IB
@@ -344,7 +345,8 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
* cost waiting for it coming back under RUNTIME only
*/
tmo_gfx = 8 * AMDGPU_IB_TEST_TIMEOUT;
- }
+ } else if (adev->gmc.xgmi.hive_id)
+ tmo_gfx = AMDGPU_IB_TEST_GFX_XGMI_TIMEOU;
for (i = 0; i < adev->num_rings; ++i) {
struct amdgpu_ring *ring = adev->rings[i];
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
- On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
> On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
>> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
>>
>> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
>> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote:
>> >>
>> >> > Hello!
>> >> >
>> >> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
>> >> > by loadable modules. The reason for this prohibition is the fact
>> >> > that using these two macros within modules requires that the size of
>> >> > the reserved region be increased, which is not something we want to
>> >> > be doing all that often. Instead, loadable modules should define an
>> >> > srcu_struct and invoke init_srcu_struct() from their module_init
>> >> > function
>> >> > and cleanup_srcu_struct() from their module_exit function. Note that
>> >> > modules using call_srcu() will also need to invoke srcu_barrier() from
>> >> > their module_exit function.
>> >>
>> >> This arbitrary API limitation seems weird.
>> >>
>> >> Isn't there a way to allow modules to use DEFINE_SRCU and
>> >> DEFINE_STATIC_SRCU
>> >> while implementing them with dynamic allocation under the hood ?
>> >
>> > Although call_srcu() already has initialization hooks, some would
>> > also be required in srcu_read_lock(), and I am concerned about adding
>> > memory allocation at that point, especially given the possibility
>> > of memory-allocation failure. And the possibility that the first
>> > srcu_read_lock() happens in an interrupt handler or similar.
>> >
>> > Or am I missing a trick here?
>>
>> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
>> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
>> would additionally lookup that section on module load, and deal with
>> those statically defined SRCU entries as if they were dynamically
>> allocated ones. It would of course cleanup those resources on module
>> unload.
>>
>> Am I missing some subtlety there ?
>
> If I understand you correctly, that is actually what is already done. The
> size of this dedicated section is currently set by PERCPU_MODULE_RESERVE,
> and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring that
> this to be increased frequently. That led to a request that something
> be done, in turn leading to this patch series.
I think we are not expressing quite the same idea.
AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data within
modules,
which ends up using percpu module reserved memory.
My idea is to make DEFINE*_SRCU have a different behavior under #ifdef MODULE.
It could emit a _global variable_ (_not_ per-cpu) within a new section. That
section would then be used by module init/exit code to figure out what "srcu
descriptors" are present in the modules. It would therefore rely on dynamic
allocation for those, therefore removing the need to involve the percpu module
reserved pool at all.
>
> I don't see a way around this short of changing module loading to do
> alloc_percpu() and then updating the relocation based on this result.
> Which would admittedly be far more convenient. I was assuming that
> this would be difficult due to varying CPU offsets or the like.
>
> But if it can be done reasonably, it would be quite a bit nicer than
> forcing dynamic allocation in cases where it is not otherwise needed.
Hopefully my explanation above helps clear out what I have in mind.
You can find similar tricks performed by include/linux/tracepoint.h:
#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
{
return offset_to_ptr(p);
}
#define __TRACEPOINT_ENTRY(name)\
asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
" .balign 4 \n" \
" .long __tracepoint_" #name " - . \n" \
" .previous \n")
#else
static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
{
return *p;
}
#define __TRACEPOINT_ENTRY(name) \
static tracepoint_ptr_t __tracepoint_ptr_##name __used \
__attribute__((section("__tracepoints_ptrs"))) = \
&__tracepoint_##name
#endif
[...]
#define DEFINE_TRACE_FN(name, reg, unreg)\
static const char __tpstrtab_##name[]\
__attribute__((section("__tracepoints_strings"))) = #name; \
struct tracepoint __tracepoint_##name\
__attribute__((section("__tracepoints"), used)) =\
{ __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
>
> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote:
> >>
> >> > Hello!
> >> >
> >> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> >> > by loadable modules. The reason for this prohibition is the fact
> >> > that using these two macros within modules requires that the size of
> >> > the reserved region be increased, which is not something we want to
> >> > be doing all that often. Instead, loadable modules should define an
> >> > srcu_struct and invoke init_srcu_struct() from their module_init function
> >> > and cleanup_srcu_struct() from their module_exit function. Note that
> >> > modules using call_srcu() will also need to invoke srcu_barrier() from
> >> > their module_exit function.
> >>
> >> This arbitrary API limitation seems weird.
> >>
> >> Isn't there a way to allow modules to use DEFINE_SRCU and
> >> DEFINE_STATIC_SRCU
> >> while implementing them with dynamic allocation under the hood ?
> >
> > Although call_srcu() already has initialization hooks, some would
> > also be required in srcu_read_lock(), and I am concerned about adding
> > memory allocation at that point, especially given the possibility
> > of memory-allocation failure. And the possibility that the first
> > srcu_read_lock() happens in an interrupt handler or similar.
> >
> > Or am I missing a trick here?
>
> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> would additionally lookup that section on module load, and deal with
> those statically defined SRCU entries as if they were dynamically
> allocated ones. It would of course cleanup those resources on module
> unload.
>
> Am I missing some subtlety there ?
If I understand you correctly, that is actually what is already done. The
size of this dedicated section is currently set by PERCPU_MODULE_RESERVE,
and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring that
this to be increased frequently. That led to a request that something
be done, in turn leading to this patch series.
I don't see a way around this short of changing module loading to do
alloc_percpu() and then updating the relocation based on this result.
Which would admittedly be far more convenient. I was assuming that
this would be difficult due to varying CPU offsets or the like.
But if it can be done reasonably, it would be quite a bit nicer than
forcing dynamic allocation in cases where it is not otherwise needed.
Thanx, Paul
> Thanks,
>
> Mathieu
>
>
> >
> > Thanx, Paul
> >
> >> Thanks,
> >>
> >> Mathieu
> >>
> >>
> >> >
> >> > This series consist of the following:
> >> >
> >> > 1. Dynamically allocate dax_srcu.
> >> >
> >> > 2. Dynamically allocate drm_unplug_srcu.
> >> >
> >> > 3. Dynamically allocate kfd_processes_srcu.
> >> >
> >> > These build and have been subjected to 0day testing, but might also need
> >> > testing by someone having the requisite hardware.
> >> >
> >> > Thanx, Paul
> >> >
> >> >
> >> >
> >> > drivers/dax/super.c| 10 +-
> >> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |5 +++
> >> > drivers/gpu/drm/amd/amdkfd/kfd_process.c |2 -
> >> > drivers/gpu/drm/drm_drv.c |8
> >> > include/linux/srcutree.h | 19 +--
> >> > kernel/rcu/rcuperf.c | 40
> >> > +++-
> >> > kernel/rcu/rcutorture.c| 48
> >> > +
> >> > 7 files changed, 105 insertions(+), 27 deletions(-)
> >>
> >> --
> >> Mathieu Desnoyers
> >> EfficiOS Inc.
> >> http://www.efficios.com
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 02, 2019 at 02:40:54PM -0400, Joel Fernandes wrote: > On Tue, Apr 02, 2019 at 08:23:34AM -0700, Paul E. McKenney wrote: > > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote: > > > - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote: > > > > > > > Hello! > > > > > > > > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU() > > > > by loadable modules. The reason for this prohibition is the fact > > > > that using these two macros within modules requires that the size of > > > > the reserved region be increased, which is not something we want to > > > > be doing all that often. Instead, loadable modules should define an > > > > srcu_struct and invoke init_srcu_struct() from their module_init > > > > function > > > > and cleanup_srcu_struct() from their module_exit function. Note that > > > > modules using call_srcu() will also need to invoke srcu_barrier() from > > > > their module_exit function. > > > > > > This arbitrary API limitation seems weird. > > > > > > Isn't there a way to allow modules to use DEFINE_SRCU and > > > DEFINE_STATIC_SRCU > > > while implementing them with dynamic allocation under the hood ? > > > > Although call_srcu() already has initialization hooks, some would > > also be required in srcu_read_lock(), and I am concerned about adding > > memory allocation at that point, especially given the possibility > > of memory-allocation failure. And the possibility that the first > > srcu_read_lock() happens in an interrupt handler or similar. > > > > Or am I missing a trick here? > > Hi Paul, > > Which 'reserved region' are you referring to? Isn't this region also > used for non-module cases in which case the same problem applies to > non-modules? The percpu/module reservation discussed in this thread: https://lore.kernel.org/lkml/c72402f2-967e-cd56-99d8-9139c9e7f...@google.com/T/#mbcacf60ddc0b3fd6e831a3ea71efc90da359a3bf For non-modules, global per-CPU variables are statically allocated. For modules, they must be dynamically allocated at modprobe time, and their size is set by PERCPU_MODULE_RESERVE. Thanx, Paul ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: fix cursor black issue
On 4/2/19 10:26 PM, tiancyin wrote:
> [Why]
> the member sdr_white_level of struct dc_cursor_attributes was not
> initialized, then the random value result that
> dcn10_set_cursor_sdr_white_level() set error hw_scale value 0x20D9(normal
> value is 0x3c00), this cause the black cursor issue.
>
> [how]
> just initilize the obj of struct dc_cursor_attributes to zero to avoid
> the random value.
>
> Change-Id: I07a53a48a33940cfb6006ed3738583f9703c7993
> Signed-off-by: Tianci Yin
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> 1 file changed, 1 insertion(+), 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 744acd8..0343ff1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5062,7 +5062,7 @@ static void handle_cursor_update(struct drm_plane
> *plane,
> struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> uint64_t address = afb ? afb->address : 0;
> struct dc_cursor_position position;
> - struct dc_cursor_attributes attributes;
> + struct dc_cursor_attributes attributes = {0};
It's probably best to make this a memset instead since we've had
compilers complain about brace / aggregate initialization before.
With that change this patch is:
Reviewed-by: Nicholas Kazlauskas
> int ret;
>
> if (!plane->state->fb && !old_plane_state->fb)
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] amdgpu_device_recover_vram always failed if only one node in shadow_list
Am 03.04.19 um 08:33 schrieb wentalou:
amdgpu_bo_restore_shadow would assign zero to r if succeeded.
r would remain zero if there is only one node in shadow_list.
current code would always return failure when r <= 0.
restart the timeout for each wait was a rather problematic bug as well.
The value of tmo SHOULD be changed, otherwise we wait tmo jiffies on each loop.
Change-Id: I7e836ec7ab6cd0f069aac24f88e454e906637541
Signed-off-by: Wentao Lou
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c4c61e9..fcb3d95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3191,11 +3191,16 @@ static int amdgpu_device_recover_vram(struct
amdgpu_device *adev)
break;
if (fence) {
- r = dma_fence_wait_timeout(fence, false, tmo);
+ tmo = dma_fence_wait_timeout(fence, false, tmo);
dma_fence_put(fence);
fence = next;
- if (r <= 0)
+ if (tmo == 0) {
+ r = -ETIMEDOUT;
break;
+ } else if (tmo < 0) {
+ r = tmo;
+ break;
+ }
} else {
fence = next;
}
@@ -3206,8 +3211,8 @@ static int amdgpu_device_recover_vram(struct
amdgpu_device *adev)
tmo = dma_fence_wait_timeout(fence, false, tmo);
dma_fence_put(fence);
- if (r <= 0 || tmo <= 0) {
- DRM_ERROR("recover vram bo from shadow failed\n");
+ if (r < 0 || tmo <= 0) {
+ DRM_ERROR("recover vram bo from shadow failed, r is %ld, tmo is
%ld\n", r, tmo);
return -EIO;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Adjust IB test timeout for XGMI configuration
Am 02.04.19 um 20:12 schrieb Liu, Shaoyun:
On XGMI configuration the ib test may tooks longer to finish
Change-Id: If3afd8eac3c342d32c387804b51fc4a4bdd35d35
Signed-off-by: shaoyunl
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 0b8ef2d..45f251f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -346,6 +346,9 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
tmo_gfx = 8 * AMDGPU_IB_TEST_TIMEOUT;
}
+ if (adev->gmc.xgmi.hive_id)
+ tmo_gfx = 2 * AMDGPU_IB_TEST_TIMEOUT;
+
Probably better to use an "else if" to note that those two mutual exclusive.
Additional to that I think it is time now to add
AMDGPU_IB_TEST_GFX_XGMI_TIMEOUT define instead of just abusing the
existing one over and over again (same of course for the SRIOV case).
Christian.
for (i = 0; i < adev->num_rings; ++i) {
struct amdgpu_ring *ring = adev->rings[i];
long tmo;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH xf86-video-amdgpu] Allow changing DCC parameters between flips
On 2019-04-02 8:13 p.m., Marek Olšák wrote: > As you probably noticed, I don't use gitlab for my own patches yet. It's not optional for xf86-video-amdgpu. Per https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/merge_requests/30#note_125584 , I ended up not using this patch, instead only allowing the DCC parameters to change with DRM minor version >= 31 as well. Thanks for this patch anyway! -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] amdgpu_device_recover_vram always failed if only one node in shadow_list
Mhm, that sounds like another bug to me which we need to investigate.
Probably a race during freeing of shadow allocations.
Christian.
Am 03.04.19 um 08:35 schrieb Lou, Wentao:
> Hi Christian,
>
> Sometimes shadow->parent would be NULL in my testbed, but not reproduce
> today...
> Just sent out another patch following your advice.
> Thanks.
>
> BR,
> Wentao
>
>
> -Original Message-
> From: Christian König
> Sent: Tuesday, April 2, 2019 6:36 PM
> To: Lou, Wentao ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] amdgpu_device_recover_vram always failed if only one
> node in shadow_list
>
> Am 02.04.19 um 11:19 schrieb wentalou:
>> amdgpu_bo_restore_shadow would assign zero to r if succeeded.
>> r would remain zero if there is only one node in shadow_list.
>> current code would always return failure when r <= 0.
>> restart the timeout for each wait was a rather problematic bug as well.
>> The value of tmo SHOULD be changed, otherwise we wait tmo jiffies on each
>> loop.
>> meanwhile, fix Call Trace by NULL of shadow->parent.
>>
>> Change-Id: I7e836ec7ab6cd0f069aac24f88e454e906637541
>> Signed-off-by: Wentao Lou
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 ++-
>>1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index c4c61e9..5a2dc44 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -3183,7 +3183,7 @@ static int amdgpu_device_recover_vram(struct
>> amdgpu_device *adev)
>>
>> /* No need to recover an evicted BO */
>> if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
>> -shadow->parent->tbo.mem.mem_type != TTM_PL_VRAM)
>> +shadow->parent == NULL || shadow->parent->tbo.mem.mem_type
>> !=
>> +TTM_PL_VRAM)
> That doesn't looks like a good idea to me. Did you actually run into this
> issue?
>
>> continue;
>>
>> r = amdgpu_bo_restore_shadow(shadow, ); @@ -3191,11
>> +3191,16
>> @@ static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
>> break;
>>
>> if (fence) {
>> -r = dma_fence_wait_timeout(fence, false, tmo);
>> +tmo = dma_fence_wait_timeout(fence, false, tmo);
>> dma_fence_put(fence);
>> fence = next;
>> -if (r <= 0)
>> +if (tmo == 0) {
>> +r = -ETIMEDOUT;
>> break;
>> +} else if (tmo < 0) {
>> +r = tmo;
>> +break;
>> +}
>> } else {
>> fence = next;
>> }
>> @@ -3206,8 +3211,8 @@ static int amdgpu_device_recover_vram(struct
>> amdgpu_device *adev)
>> tmo = dma_fence_wait_timeout(fence, false, tmo);
>> dma_fence_put(fence);
>>
>> -if (r <= 0 || tmo <= 0) {
>> -DRM_ERROR("recover vram bo from shadow failed\n");
>> +if (r < 0 || tmo <= 0) {
>> +DRM_ERROR("recover vram bo from shadow failed, tmo is %d\n",
>> tmo);
> Maybe print both r and tmo in the message.
>
> Regards,
> Christian.
>
>> return -EIO;
>> }
>>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
How to get useful information other than "the whole system locks up"?
Hi, I have a Sapphire Technology Hawaii XT (R9 290X) using amdgpu driver with kernel 5.1.0-rc3. The issue happens with current 4.19.0 debian testing, 4.20-trunk, 5.0.0-trunk and rc2 and 3. It usually happens when I'm reproducing video, but I haven't figured out a way to reproduce it. It happened once without reproducing. I'm aware that the support is experimental, but radeon driver doesn't seems capable of direct rendering on this card dropping to llvmepipe. I had a ssh server installed in case I could log in while it crashes, and the only relevant line I found was: drm:amdgpu job timeout [amdgpu]] **ERROR** ring gfx timeout, signaled seq=399919, emitted seq=399921 But that turned several bug reports which seems to have been fixed and the context and symptoms are too different to mine. I have tried forcing the amdgpu xorg driver with same results (was using radeon). -- Braiam ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] amdgpu_device_recover_vram always failed if only one node in shadow_list
Hi Christian,
Sometimes shadow->parent would be NULL in my testbed, but not reproduce today...
Just sent out another patch following your advice.
Thanks.
BR,
Wentao
-Original Message-
From: Christian König
Sent: Tuesday, April 2, 2019 6:36 PM
To: Lou, Wentao ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] amdgpu_device_recover_vram always failed if only one node
in shadow_list
Am 02.04.19 um 11:19 schrieb wentalou:
> amdgpu_bo_restore_shadow would assign zero to r if succeeded.
> r would remain zero if there is only one node in shadow_list.
> current code would always return failure when r <= 0.
> restart the timeout for each wait was a rather problematic bug as well.
> The value of tmo SHOULD be changed, otherwise we wait tmo jiffies on each
> loop.
> meanwhile, fix Call Trace by NULL of shadow->parent.
>
> Change-Id: I7e836ec7ab6cd0f069aac24f88e454e906637541
> Signed-off-by: Wentao Lou
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c4c61e9..5a2dc44 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3183,7 +3183,7 @@ static int amdgpu_device_recover_vram(struct
> amdgpu_device *adev)
>
> /* No need to recover an evicted BO */
> if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
> - shadow->parent->tbo.mem.mem_type != TTM_PL_VRAM)
> + shadow->parent == NULL || shadow->parent->tbo.mem.mem_type
> !=
> +TTM_PL_VRAM)
That doesn't looks like a good idea to me. Did you actually run into this issue?
> continue;
>
> r = amdgpu_bo_restore_shadow(shadow, ); @@ -3191,11
> +3191,16
> @@ static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
> break;
>
> if (fence) {
> - r = dma_fence_wait_timeout(fence, false, tmo);
> + tmo = dma_fence_wait_timeout(fence, false, tmo);
> dma_fence_put(fence);
> fence = next;
> - if (r <= 0)
> + if (tmo == 0) {
> + r = -ETIMEDOUT;
> break;
> + } else if (tmo < 0) {
> + r = tmo;
> + break;
> + }
> } else {
> fence = next;
> }
> @@ -3206,8 +3211,8 @@ static int amdgpu_device_recover_vram(struct
> amdgpu_device *adev)
> tmo = dma_fence_wait_timeout(fence, false, tmo);
> dma_fence_put(fence);
>
> - if (r <= 0 || tmo <= 0) {
> - DRM_ERROR("recover vram bo from shadow failed\n");
> + if (r < 0 || tmo <= 0) {
> + DRM_ERROR("recover vram bo from shadow failed, tmo is %d\n",
> tmo);
Maybe print both r and tmo in the message.
Regards,
Christian.
> return -EIO;
> }
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] amdgpu_device_recover_vram always failed if only one node in shadow_list
amdgpu_bo_restore_shadow would assign zero to r if succeeded.
r would remain zero if there is only one node in shadow_list.
current code would always return failure when r <= 0.
restart the timeout for each wait was a rather problematic bug as well.
The value of tmo SHOULD be changed, otherwise we wait tmo jiffies on each loop.
Change-Id: I7e836ec7ab6cd0f069aac24f88e454e906637541
Signed-off-by: Wentao Lou
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c4c61e9..fcb3d95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3191,11 +3191,16 @@ static int amdgpu_device_recover_vram(struct
amdgpu_device *adev)
break;
if (fence) {
- r = dma_fence_wait_timeout(fence, false, tmo);
+ tmo = dma_fence_wait_timeout(fence, false, tmo);
dma_fence_put(fence);
fence = next;
- if (r <= 0)
+ if (tmo == 0) {
+ r = -ETIMEDOUT;
break;
+ } else if (tmo < 0) {
+ r = tmo;
+ break;
+ }
} else {
fence = next;
}
@@ -3206,8 +3211,8 @@ static int amdgpu_device_recover_vram(struct
amdgpu_device *adev)
tmo = dma_fence_wait_timeout(fence, false, tmo);
dma_fence_put(fence);
- if (r <= 0 || tmo <= 0) {
- DRM_ERROR("recover vram bo from shadow failed\n");
+ if (r < 0 || tmo <= 0) {
+ DRM_ERROR("recover vram bo from shadow failed, r is %ld, tmo is
%ld\n", r, tmo);
return -EIO;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amdgpu: Make default ras error type to none
Unless IP has implemented its own ras, use ERROR_NONE as the default
type.
Signed-off-by: xinhui pan
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index fc4bf7237d4b..655d58b63405 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -685,11 +685,13 @@ static int amdgpu_ras_enable_all_features(struct
amdgpu_device *adev,
struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
int ras_block_count = AMDGPU_RAS_BLOCK_COUNT;
int i;
+ const enum amdgpu_ras_error_type default_ras_type =
+ AMDGPU_RAS_ERROR__NONE;
for (i = 0; i < ras_block_count; i++) {
struct ras_common_if head = {
.block = i,
- .type = AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE,
+ .type = default_ras_type,
.sub_block_index = 0,
};
strcpy(head.name, ras_block_str(i));
@@ -1495,9 +1497,6 @@ int amdgpu_ras_init(struct amdgpu_device *adev)
amdgpu_ras_mask &= AMDGPU_RAS_BLOCK_MASK;
- if (con->flags & AMDGPU_RAS_FLAG_INIT_BY_VBIOS)
- amdgpu_ras_enable_all_features(adev, 1);
-
if (amdgpu_ras_fs_init(adev))
goto fs_out;
@@ -1525,18 +1524,25 @@ void amdgpu_ras_post_init(struct amdgpu_device *adev)
if (!con)
return;
- /* We enable ras on all hw_supported block, but as boot parameter might
-* disable some of them and one or more IP has not implemented yet.
-* So we disable them on behalf.
-*/
if (con->flags & AMDGPU_RAS_FLAG_INIT_BY_VBIOS) {
+ /* Set up all other IPs which are not implemented. There is a
+* tricky thing that IP's actual ras error type should be
+* MULTI_UNCORRECTABLE, but as driver does not handle it, so
+* ERROR_NONE make sense anyway.
+*/
+ amdgpu_ras_enable_all_features(adev, 1);
+
+ /* We enable ras on all hw_supported block, but as boot
+* parameter might disable some of them and one or more IP has
+* not implemented yet. So we disable them on behalf.
+*/
list_for_each_entry_safe(obj, tmp, >head, node) {
if (!amdgpu_ras_is_supported(adev, obj->head.block)) {
amdgpu_ras_feature_enable(adev, >head, 0);
/* there should be no any reference. */
WARN_ON(alive_obj(obj));
}
- };
+ }
}
}
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amdgpu: Allow IP to skip the first ras enablement
If vbios has enabled ras for us, skip it
Signed-off-by: xinhui pan
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 2b7a420d5a1f..fc4bf7237d4b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -118,8 +118,11 @@ const char *ras_block_string[] = {
#define ras_err_str(i) (ras_error_string[ffs(i)])
#define ras_block_str(i) (ras_block_string[i])
-#define AMDGPU_RAS_FLAG_INIT_BY_VBIOS 1
-#define RAS_DEFAULT_FLAGS (AMDGPU_RAS_FLAG_INIT_BY_VBIOS)
+#define AMDGPU_RAS_FLAG(x) ({BUILD_BUG_ON(BIT(x) & AMDGPU_RAS_BLOCK_MASK);\
+ BIT(x); })
+#define AMDGPU_RAS_FLAG_INIT_BY_VBIOS AMDGPU_RAS_FLAG(31)
+#define RAS_DEFAULT_FLAGS (AMDGPU_RAS_FLAG_INIT_BY_VBIOS |\
+ AMDGPU_RAS_BLOCK_MASK)
static int amdgpu_ras_reserve_vram(struct amdgpu_device *adev,
uint64_t offset, uint64_t size,
@@ -551,6 +554,20 @@ static int amdgpu_ras_is_feature_enabled(struct
amdgpu_device *adev,
return con->features & BIT(head->block);
}
+static bool amdgpu_ras_cmpxchg_feature_initialized(struct amdgpu_device *adev,
+ struct ras_common_if *head, bool new)
+{
+ struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
+ bool old = (con->flags & AMDGPU_RAS_FLAG_INIT_BY_VBIOS) &&
+ (con->flags & BIT(head->block));
+
+ if (new)
+ con->flags |= BIT(head->block);
+ else
+ con->flags &= ~BIT(head->block);
+
+ return old;
+}
/*
* if obj is not created, then create one.
* set feature enable flag.
@@ -620,6 +637,9 @@ int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
/* Are we alerady in that state we are going to set? */
if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
return 0;
+ /* check if vbios has enabled ras for us on the block. */
+ if (enable && amdgpu_ras_cmpxchg_feature_initialized(adev, head, false))
+ goto first_time_bypass_psp;
ret = psp_ras_enable_features(>psp, , enable);
if (ret) {
@@ -630,6 +650,7 @@ int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
return -EINVAL;
}
+first_time_bypass_psp:
/* setup the obj */
__amdgpu_ras_feature_enable(adev, head, enable);
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
