Op 20-04-16 om 04:26 schreef Matt Roper:
> In an upcoming patch we'll move this calculation to the atomic 'check'
> phase so that the display update can be rejected early if no valid
> watermark programming is possible.
>
> v2:
> - Drop intel_pstate_for_cstate_plane() helper and add note about how
> the code needs to evolve in the future if we start allowing more than
> one pending commit against a CRTC. (Maarten)
>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++++++++----------
> 1 file changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0c0a312..d114375 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3330,14 +3330,17 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> return true;
> }
>
> -static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
> - struct skl_ddb_allocation *ddb,
> - struct intel_crtc_state *cstate,
> - int level,
> - struct skl_wm_level *result)
> +static int
> +skl_compute_wm_level(const struct drm_i915_private *dev_priv,
> + struct skl_ddb_allocation *ddb,
> + struct intel_crtc_state *cstate,
> + int level,
> + struct skl_wm_level *result)
> {
> struct drm_device *dev = dev_priv->dev;
> + struct drm_atomic_state *state = cstate->base.state;
> struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> + struct drm_plane *plane;
> struct intel_plane *intel_plane;
> struct intel_plane_state *intel_pstate;
> uint16_t ddb_blocks;
> @@ -3346,7 +3349,29 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> int i = skl_wm_plane_id(intel_plane);
>
> - intel_pstate = to_intel_plane_state(intel_plane->base.state);
> + plane = &intel_plane->base;
> + intel_pstate = NULL;
> + if (state)
> + intel_pstate =
> + intel_atomic_get_existing_plane_state(state,
> + intel_plane);
> +
> + /*
> + * Note: If we start supporting multiple pending atomic commits
> + * against the same planes/CRTC's in the future, plane->state
> + * will no longer be the correct pre-state to use for the
> + * calculations here and we'll need to change where we get the
> + * 'unchanged' plane data from.
> + *
> + * For now this is fine because we only allow one queued commit
> + * against a CRTC. Even if the plane isn't modified by this
> + * transaction and we don't have a plane lock, we still have
> + * the CRTC's lock, so we know that no other transactions are
> + * racing with us to update it.
> + */
> + if (!intel_pstate)
> + intel_pstate = to_intel_plane_state(plane->state);
>
You can make this race-free by using using for_each_intel_plane_mask
with crtc_state->plane_mask ¦ old_crtc_state->plane_mask.
Other planes can be assumed to have a data rate of 0, and can unfortunately be updated
in parallel. Since you can only update atomic properties that are not ->crtc or ->fb it wouldn't
change anything visibly on the screen, but it would cause your plane->state to be freed behind
your back if you're not careful.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
