To make NV12 working on icl, we need to update 2 planes simultaneously. I've chosen to do this in the CRTC step after plane validation is done, so we know what planes are (in)visible. The linked Y plane will get updated in intel_plane_update_planes_on_crtc(), by the call to update_slave, which gets the master's plane_state as argument.
The link requires both planes for atomic_update to work, so make sure skl_ddb_add_affected_planes() adds both states. Changes since v1: - Introduce icl_is_nv12_y_plane(), instead of hardcoding sprite numbers. - Put all the state updating login in intel_plane_atomic_check_with_state(). - Clean up changes in intel_plane_atomic_check(). Signed-off-by: Maarten Lankhorst <[email protected]> --- drivers/gpu/drm/i915/intel_atomic_plane.c | 107 ++++++++++++++++++---- drivers/gpu/drm/i915/intel_display.c | 57 ++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 53 +++++++++++ drivers/gpu/drm/i915/intel_pm.c | 12 ++- 4 files changed, 210 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index d754a8ba9ffb..4f224f71d50e 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -117,7 +117,12 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ struct intel_plane *intel_plane = to_intel_plane(plane); int ret; + crtc_state->active_planes &= ~BIT(intel_plane->id); intel_state->base.visible = false; + intel_state->linked_plane = NULL; + intel_state->slave = false; + + /* If this is a cursor or Y plane, no further checks are needed. */ if (!intel_state->base.crtc && !old_plane_state->base.crtc) return 0; @@ -128,8 +133,6 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ /* FIXME pre-g4x don't work like this */ if (state->visible) crtc_state->active_planes |= BIT(intel_plane->id); - else - crtc_state->active_planes &= ~BIT(intel_plane->id); if (state->visible && state->fb->format->format == DRM_FORMAT_NV12) crtc_state->nv12_planes |= BIT(intel_plane->id); @@ -142,27 +145,76 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ state); } -static int intel_plane_atomic_check(struct drm_plane *plane, - struct drm_plane_state *new_plane_state) +static int intel_plane_atomic_check(struct drm_plane *drm_plane, + struct drm_plane_state *new_drm_plane_state) { - struct drm_atomic_state *state = new_plane_state->state; - const struct drm_plane_state *old_plane_state = - drm_atomic_get_old_plane_state(state, plane); - struct drm_crtc *crtc = new_plane_state->crtc ?: old_plane_state->crtc; - const struct drm_crtc_state *old_crtc_state; - struct drm_crtc_state *new_crtc_state; - - new_plane_state->visible = false; + struct intel_atomic_state *state = + to_intel_atomic_state(new_drm_plane_state->state); + struct intel_plane *plane = to_intel_plane(drm_plane); + const struct intel_plane_state *old_plane_state = + intel_atomic_get_old_plane_state(state, plane); + struct intel_plane_state *new_plane_state = + to_intel_plane_state(new_drm_plane_state); + struct intel_crtc *crtc = to_intel_crtc( + new_plane_state->base.crtc ?: + old_plane_state->base.crtc); + const struct intel_crtc_state *old_crtc_state; + struct intel_crtc_state *new_crtc_state; + struct intel_plane *linked = old_plane_state->linked_plane; + int ret; + const struct intel_plane_state *old_linked_state; + struct intel_plane_state *new_linked_state = NULL; + + if (linked) { + /* + * Make sure a previously linked plane (and implicitly, the CRTC) + * is part of the atomic commit. + */ + if (!intel_atomic_get_new_plane_state(state, linked)) { + new_linked_state = intel_atomic_get_plane_state(state, linked); + if (IS_ERR(new_linked_state)) + return PTR_ERR(new_linked_state); + } + + old_linked_state = + intel_atomic_get_old_plane_state(state, linked); + + /* + * This will happen when we're the Y plane. In which case + * old/new_state->crtc are both NULL. We still need to perform + * updates on the linked plane. + */ + if (!crtc) + crtc = to_intel_crtc(old_linked_state->base.crtc); + + WARN_ON(!crtc); + } + + new_plane_state->base.visible = false; if (!crtc) return 0; - old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc); - new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); + new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); + + if (new_linked_state && + drm_plane_index(&linked->base) < drm_plane_index(&plane->base)) { + /* + * This function is called from drm_atomic_helper_check_planes(), which + * will normally check the newly added plane for us, but since we're + * already in that function, it won't check the plane if our index + * is bigger than the linked index because of the + * for_each_oldnew_plane_in_state() call. + */ + new_crtc_state->base.planes_changed = true; + ret = intel_plane_atomic_check_with_state(old_crtc_state, new_crtc_state, + old_linked_state, new_linked_state); + if (ret) + return ret; + } - return intel_plane_atomic_check_with_state(to_intel_crtc_state(old_crtc_state), - to_intel_crtc_state(new_crtc_state), - to_intel_plane_state(old_plane_state), - to_intel_plane_state(new_plane_state)); + return intel_plane_atomic_check_with_state(old_crtc_state, new_crtc_state, + old_plane_state, new_plane_state); } void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, @@ -187,6 +239,25 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, trace_intel_update_plane(&plane->base, crtc); plane->update_plane(plane, new_crtc_state, new_plane_state); + } else if (new_plane_state->slave) { + struct intel_plane *master = + new_plane_state->linked_plane; + + /* + * We update the slave plane from this function because + * programming it from the master plane's update_plane + * callback runs into issues when the Y plane is + * reassigned, disabled or used by a different plane. + * + * The slave plane is updated with the master plane's + * plane_state. + */ + new_plane_state = + intel_atomic_get_new_plane_state(old_state, master); + + trace_intel_update_plane(&plane->base, crtc); + + plane->update_slave(plane, new_crtc_state, new_plane_state); } else { trace_intel_disable_plane(&plane->base, crtc); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 122f3f4cfc53..a53953df6c9f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10736,6 +10736,60 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state, return true; } +static int icl_check_nv12_planes(struct drm_i915_private *dev_priv, + struct intel_crtc *crtc, + struct intel_crtc_state *crtc_state) +{ + struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->base.state); + struct intel_plane *plane, *aux; + + if (!crtc_state->nv12_planes) + return 0; + + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) { + struct intel_plane_state *plane_state, *aux_state; + struct drm_plane_state *drm_aux_state = NULL; + + if (!(crtc_state->nv12_planes & BIT(plane->id))) + continue; + + plane_state = intel_atomic_get_new_plane_state(state, plane); + if (!plane_state) + continue; + + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, aux) { + if (!icl_is_nv12_y_plane(aux->id)) + continue; + + if (crtc_state->active_planes & BIT(aux->id)) + continue; + + drm_aux_state = drm_atomic_get_plane_state(&state->base, &aux->base); + if (IS_ERR(drm_aux_state)) + return PTR_ERR(drm_aux_state); + + break; + } + + if (!drm_aux_state) { + DRM_DEBUG_KMS("Need %d free Y planes for NV12\n", + hweight8(crtc_state->nv12_planes)); + + return -EINVAL; + } + + plane_state->linked_plane = aux; + + aux_state = to_intel_plane_state(drm_aux_state); + aux_state->slave = true; + aux_state->linked_plane = plane; + crtc_state->active_planes |= BIT(aux->id); + DRM_DEBUG_KMS("Using %s as Y plane for %s\n", aux->base.name, plane->base.name); + } + + return 0; +} + static int intel_crtc_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *crtc_state) { @@ -10807,6 +10861,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, if (mode_changed) ret = skl_update_scaler_crtc(pipe_config); + if (!ret) + ret = icl_check_nv12_planes(dev_priv, intel_crtc, + pipe_config); if (!ret) ret = skl_check_pipe_max_pixel_rate(intel_crtc, pipe_config); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8073a85d7178..dd9600065eff 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -539,6 +539,26 @@ struct intel_plane_state { */ int scaler_id; + /* + * linked_plane: + * + * ICL planar formats require 2 planes that are updated as pairs. + * This member is used to make sure the other plane is also updated + * when required, and for update_slave() to find the correct + * plane_state to pass as argument. + */ + struct intel_plane *linked_plane; + + /* + * slave: + * If set don't update use the linked plane's state for updating + * this plane during atomic commit with the update_slave() callback. + * + * It's also used by the watermark code to ignore wm calculations on + * this plane. They're calculated by the linked plane's wm code. + */ + bool slave; + struct drm_intel_sprite_colorkey ckey; }; @@ -973,6 +993,9 @@ struct intel_plane { void (*update_plane)(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state); + void (*update_slave)(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state); void (*disable_plane)(struct intel_plane *plane, struct intel_crtc *crtc); bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); @@ -1330,6 +1353,27 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi) return container_of(intel_hdmi, struct intel_digital_port, hdmi); } +static inline struct intel_plane_state * +intel_atomic_get_plane_state(struct intel_atomic_state *state, + struct intel_plane *plane) +{ + struct drm_plane_state *ret = + drm_atomic_get_plane_state(&state->base, &plane->base); + + if (IS_ERR(ret)) + return ERR_CAST(ret); + + return to_intel_plane_state(ret); +} + +static inline struct intel_plane_state * +intel_atomic_get_old_plane_state(struct intel_atomic_state *state, + struct intel_plane *plane) +{ + return to_intel_plane_state(drm_atomic_get_new_plane_state(&state->base, + &plane->base)); +} + static inline struct intel_plane_state * intel_atomic_get_new_plane_state(struct intel_atomic_state *state, struct intel_plane *plane) @@ -2143,6 +2187,15 @@ int skl_plane_check(struct intel_crtc_state *crtc_state, int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state); int chv_plane_check_rotation(const struct intel_plane_state *plane_state); +static inline bool icl_is_nv12_y_plane(enum plane_id id) +{ + /* Don't need to do a gen check, these planes are only available on gen11 */ + if (id == PLANE_SPRITE4 || id == PLANE_SPRITE5) + return true; + + return false; +} + /* intel_tv.c */ void intel_tv_init(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1db9b8328275..d76d93452137 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5137,11 +5137,12 @@ skl_ddb_add_affected_planes(struct intel_crtc_state *cstate) struct intel_atomic_state *intel_state = to_intel_atomic_state(state); struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb; struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb; - struct drm_plane_state *plane_state; struct drm_plane *plane; enum pipe pipe = intel_crtc->pipe; drm_for_each_plane_mask(plane, dev, cstate->base.plane_mask) { + struct drm_plane_state *plane_state; + struct intel_plane *linked; enum plane_id plane_id = to_intel_plane(plane)->id; if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][plane_id], @@ -5153,6 +5154,15 @@ skl_ddb_add_affected_planes(struct intel_crtc_state *cstate) plane_state = drm_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) return PTR_ERR(plane_state); + + /* Make sure linked plane is updated too */ + linked = to_intel_plane_state(plane_state)->linked_plane; + if (!linked) + continue; + + plane_state = drm_atomic_get_plane_state(state, &linked->base); + if (IS_ERR(plane_state)) + return PTR_ERR(plane_state); } return 0; -- 2.18.0 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
