Skip to content

Commit

Permalink
Merge tag 'drm-intel-fixes-2024-04-10' of https://anongit.freedesktop…
Browse files Browse the repository at this point in the history
….org/git/drm/drm-intel into drm-fixes

Display fixes:
- Couple CDCLK programming fixes (Ville)
- HDCP related fix (Suraj)
- 4 Bigjoiner related fixes (Ville)

Core fix:
- Fix for a circular locking around GuC on reset+wedged case (John)

Signed-off-by: Dave Airlie <[email protected]>

From: Rodrigo Vivi <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
  • Loading branch information
airlied committed Apr 11, 2024
2 parents 718c4fb + dcd8992 commit aaf00e6
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 27 deletions.
42 changes: 31 additions & 11 deletions drivers/gpu/drm/i915/display/intel_cdclk.c
Original file line number Diff line number Diff line change
Expand Up @@ -2534,7 +2534,8 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state)
intel_atomic_get_old_cdclk_state(state);
const struct intel_cdclk_state *new_cdclk_state =
intel_atomic_get_new_cdclk_state(state);
enum pipe pipe = new_cdclk_state->pipe;
struct intel_cdclk_config cdclk_config;
enum pipe pipe;

if (!intel_cdclk_changed(&old_cdclk_state->actual,
&new_cdclk_state->actual))
Expand All @@ -2543,12 +2544,25 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state)
if (IS_DG2(i915))
intel_cdclk_pcode_pre_notify(state);

if (pipe == INVALID_PIPE ||
old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) {
drm_WARN_ON(&i915->drm, !new_cdclk_state->base.changed);
if (new_cdclk_state->disable_pipes) {
cdclk_config = new_cdclk_state->actual;
pipe = INVALID_PIPE;
} else {
if (new_cdclk_state->actual.cdclk >= old_cdclk_state->actual.cdclk) {
cdclk_config = new_cdclk_state->actual;
pipe = new_cdclk_state->pipe;
} else {
cdclk_config = old_cdclk_state->actual;
pipe = INVALID_PIPE;
}

intel_set_cdclk(i915, &new_cdclk_state->actual, pipe);
cdclk_config.voltage_level = max(new_cdclk_state->actual.voltage_level,
old_cdclk_state->actual.voltage_level);
}

drm_WARN_ON(&i915->drm, !new_cdclk_state->base.changed);

intel_set_cdclk(i915, &cdclk_config, pipe);
}

/**
Expand All @@ -2566,7 +2580,7 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state)
intel_atomic_get_old_cdclk_state(state);
const struct intel_cdclk_state *new_cdclk_state =
intel_atomic_get_new_cdclk_state(state);
enum pipe pipe = new_cdclk_state->pipe;
enum pipe pipe;

if (!intel_cdclk_changed(&old_cdclk_state->actual,
&new_cdclk_state->actual))
Expand All @@ -2575,12 +2589,15 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state)
if (IS_DG2(i915))
intel_cdclk_pcode_post_notify(state);

if (pipe != INVALID_PIPE &&
old_cdclk_state->actual.cdclk > new_cdclk_state->actual.cdclk) {
drm_WARN_ON(&i915->drm, !new_cdclk_state->base.changed);
if (!new_cdclk_state->disable_pipes &&
new_cdclk_state->actual.cdclk < old_cdclk_state->actual.cdclk)
pipe = new_cdclk_state->pipe;
else
pipe = INVALID_PIPE;

drm_WARN_ON(&i915->drm, !new_cdclk_state->base.changed);

intel_set_cdclk(i915, &new_cdclk_state->actual, pipe);
}
intel_set_cdclk(i915, &new_cdclk_state->actual, pipe);
}

static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state *crtc_state)
Expand Down Expand Up @@ -3058,6 +3075,7 @@ static struct intel_global_state *intel_cdclk_duplicate_state(struct intel_globa
return NULL;

cdclk_state->pipe = INVALID_PIPE;
cdclk_state->disable_pipes = false;

return &cdclk_state->base;
}
Expand Down Expand Up @@ -3236,6 +3254,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
if (ret)
return ret;

new_cdclk_state->disable_pipes = true;

drm_dbg_kms(&dev_priv->drm,
"Modeset required for cdclk change\n");
}
Expand Down
3 changes: 3 additions & 0 deletions drivers/gpu/drm/i915/display/intel_cdclk.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ struct intel_cdclk_state {

/* bitmask of active pipes */
u8 active_pipes;

/* update cdclk with pipes disabled */
bool disable_pipes;
};

int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
Expand Down
5 changes: 5 additions & 0 deletions drivers/gpu/drm/i915/display/intel_ddi.c
Original file line number Diff line number Diff line change
Expand Up @@ -4256,7 +4256,12 @@ static bool m_n_equal(const struct intel_link_m_n *m_n_1,
static bool crtcs_port_sync_compatible(const struct intel_crtc_state *crtc_state1,
const struct intel_crtc_state *crtc_state2)
{
/*
* FIXME the modeset sequence is currently wrong and
* can't deal with bigjoiner + port sync at the same time.
*/
return crtc_state1->hw.active && crtc_state2->hw.active &&
!crtc_state1->bigjoiner_pipes && !crtc_state2->bigjoiner_pipes &&
crtc_state1->output_types == crtc_state2->output_types &&
crtc_state1->output_format == crtc_state2->output_format &&
crtc_state1->lane_count == crtc_state2->lane_count &&
Expand Down
6 changes: 5 additions & 1 deletion drivers/gpu/drm/i915/display/intel_dp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2725,7 +2725,11 @@ intel_dp_drrs_compute_config(struct intel_connector *connector,
intel_panel_downclock_mode(connector, &pipe_config->hw.adjusted_mode);
int pixel_clock;

if (has_seamless_m_n(connector))
/*
* FIXME all joined pipes share the same transcoder.
* Need to account for that when updating M/N live.
*/
if (has_seamless_m_n(connector) && !pipe_config->bigjoiner_pipes)
pipe_config->update_m_n = true;

if (!can_enable_drrs(connector, pipe_config, downclock_mode)) {
Expand Down
5 changes: 4 additions & 1 deletion drivers/gpu/drm/i915/display/intel_dp_hdcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -691,12 +691,15 @@ int intel_dp_hdcp_get_remote_capability(struct intel_connector *connector,
u8 bcaps;
int ret;

*hdcp_capable = false;
*hdcp2_capable = false;
if (!intel_encoder_is_mst(connector->encoder))
return -EINVAL;

ret = _intel_dp_hdcp2_get_capability(aux, hdcp2_capable);
if (ret)
return ret;
drm_dbg_kms(&i915->drm,
"HDCP2 DPCD capability read failed err: %d\n", ret);

ret = intel_dp_hdcp_read_bcaps(aux, i915, &bcaps);
if (ret)
Expand Down
11 changes: 11 additions & 0 deletions drivers/gpu/drm/i915/display/intel_psr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,17 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
return;
}

/*
* FIXME figure out what is wrong with PSR+bigjoiner and
* fix it. Presumably something related to the fact that
* PSR is a transcoder level feature.
*/
if (crtc_state->bigjoiner_pipes) {
drm_dbg_kms(&dev_priv->drm,
"PSR disabled due to bigjoiner\n");
return;
}

if (CAN_PANEL_REPLAY(intel_dp))
crtc_state->has_panel_replay = true;
else
Expand Down
7 changes: 7 additions & 0 deletions drivers/gpu/drm/i915/display/intel_vrr.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
const struct drm_display_info *info = &connector->base.display_info;
int vmin, vmax;

/*
* FIXME all joined pipes share the same transcoder.
* Need to account for that during VRR toggle/push/etc.
*/
if (crtc_state->bigjoiner_pipes)
return;

if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
return;

Expand Down
23 changes: 9 additions & 14 deletions drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
Original file line number Diff line number Diff line change
Expand Up @@ -1403,14 +1403,17 @@ static void guc_cancel_busyness_worker(struct intel_guc *guc)
* Trying to pass a 'need_sync' or 'in_reset' flag all the way down through
* every possible call stack is unfeasible. It would be too intrusive to many
* areas that really don't care about the GuC backend. However, there is the
* 'reset_in_progress' flag available, so just use that.
* I915_RESET_BACKOFF flag and the gt->reset.mutex can be tested for is_locked.
* So just use those. Note that testing both is required due to the hideously
* complex nature of the i915 driver's reset code paths.
*
* And note that in the case of a reset occurring during driver unload
* (wedge_on_fini), skipping the cancel in _prepare (when the reset flag is set
* is fine because there is another cancel in _finish (when the reset flag is
* not).
* (wedged_on_fini), skipping the cancel in reset_prepare/reset_fini (when the
* reset flag/mutex are set) is fine because there is another explicit cancel in
* intel_guc_submission_fini (when the reset flag/mutex are not).
*/
if (guc_to_gt(guc)->uc.reset_in_progress)
if (mutex_is_locked(&guc_to_gt(guc)->reset.mutex) ||
test_bit(I915_RESET_BACKOFF, &guc_to_gt(guc)->reset.flags))
cancel_delayed_work(&guc->timestamp.work);
else
cancel_delayed_work_sync(&guc->timestamp.work);
Expand All @@ -1424,8 +1427,6 @@ static void __reset_guc_busyness_stats(struct intel_guc *guc)
unsigned long flags;
ktime_t unused;

guc_cancel_busyness_worker(guc);

spin_lock_irqsave(&guc->timestamp.lock, flags);

guc_update_pm_timestamp(guc, &unused);
Expand Down Expand Up @@ -2004,13 +2005,6 @@ void intel_guc_submission_cancel_requests(struct intel_guc *guc)

void intel_guc_submission_reset_finish(struct intel_guc *guc)
{
/*
* Ensure the busyness worker gets cancelled even on a fatal wedge.
* Note that reset_prepare is not allowed to because it confuses lockdep.
*/
if (guc_submission_initialized(guc))
guc_cancel_busyness_worker(guc);

/* Reset called during driver load or during wedge? */
if (unlikely(!guc_submission_initialized(guc) ||
!intel_guc_is_fw_running(guc) ||
Expand Down Expand Up @@ -2136,6 +2130,7 @@ void intel_guc_submission_fini(struct intel_guc *guc)
if (!guc->submission_initialized)
return;

guc_fini_engine_stats(guc);
guc_flush_destroyed_contexts(guc);
guc_lrc_desc_pool_destroy_v69(guc);
i915_sched_engine_put(guc->sched_engine);
Expand Down
4 changes: 4 additions & 0 deletions drivers/gpu/drm/i915/gt/uc/intel_uc.c
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,10 @@ void intel_uc_reset_finish(struct intel_uc *uc)
{
struct intel_guc *guc = &uc->guc;

/*
* NB: The wedge code path results in prepare -> prepare -> finish -> finish.
* So this function is sometimes called with the in-progress flag not set.
*/
uc->reset_in_progress = false;

/* Firmware expected to be running when this function is called */
Expand Down

0 comments on commit aaf00e6

Please sign in to comment.