On Tue, Aug 15, 2023 at 03:57:09PM -0300, André Almeida wrote:
> Given that prop changes may lead to modesetting, which would defeat the
> fast path of the async flip, refuse any atomic prop change for async
> flips in atomic API. The only exceptions are the framebuffer ID to flip
> to and the mode ID, that could be referring to an identical mode.
FYI, the solid fill series adds an enum drm_plane_pixel_source and and a
new solid fill pixel source. Changing the solid fill color would be
effectively the same as changing the FB_ID. On the other hand, changing
the FB_ID no longer necessarily results in an update when the pixel
source is set to solid fill.
> Signed-off-by: André Almeida <andrealmeid@xxxxxxxxxx>
> ---
> v5: no changes
> v4: new patch
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 5 +++
> drivers/gpu/drm/drm_atomic_uapi.c | 52 +++++++++++++++++++++++++++--
> drivers/gpu/drm/drm_crtc_internal.h | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 2 +-
> 4 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 292e38eb6218..b34e3104afd1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -629,6 +629,11 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>
> if (!drm_mode_equal(&old_crtc_state->mode, &new_crtc_state->mode)) {
> + if (new_crtc_state->async_flip) {
> + drm_dbg_atomic(dev, "[CRTC:%d:%s] no mode changes allowed during async flip\n",
> + crtc->base.id, crtc->name);
> + return -EINVAL;
> + }
> drm_dbg_atomic(dev, "[CRTC:%d:%s] mode changed\n",
> crtc->base.id, crtc->name);
> new_crtc_state->mode_changed = true;
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index a15121e75a0a..6c423a7e8c7b 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1006,13 +1006,28 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state,
> return ret;
> }
>
> +static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value,
> + struct drm_property *prop)
> +{
> + if (ret != 0 || old_val != prop_value) {
> + drm_dbg_atomic(prop->dev,
> + "[PROP:%d:%s] No prop can be changed during async flip\n",
> + prop->base.id, prop->name);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> int drm_atomic_set_property(struct drm_atomic_state *state,
> struct drm_file *file_priv,
> struct drm_mode_object *obj,
> struct drm_property *prop,
> - uint64_t prop_value)
> + uint64_t prop_value,
> + bool async_flip)
> {
> struct drm_mode_object *ref;
> + uint64_t old_val;
> int ret;
>
> if (!drm_property_change_valid_get(prop, prop_value, &ref))
> @@ -1029,6 +1044,13 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
> break;
> }
>
> + if (async_flip) {
> + ret = drm_atomic_connector_get_property(connector, connector_state,
> + prop, &old_val);
> + ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);
> + break;
> + }
> +
> ret = drm_atomic_connector_set_property(connector,
> connector_state, file_priv,
> prop, prop_value);
> @@ -1037,6 +1059,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
> case DRM_MODE_OBJECT_CRTC: {
> struct drm_crtc *crtc = obj_to_crtc(obj);
> struct drm_crtc_state *crtc_state;
> + struct drm_mode_config *config = &crtc->dev->mode_config;
>
> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> if (IS_ERR(crtc_state)) {
> @@ -1044,6 +1067,18 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
> break;
> }
>
> + /*
> + * We allow mode_id changes here for async flips, because we
> + * check later on drm_atomic_helper_check_modeset() callers if
> + * there are modeset changes or they are equal
> + */
> + if (async_flip && prop != config->prop_mode_id) {
> + ret = drm_atomic_crtc_get_property(crtc, crtc_state,
> + prop, &old_val);
> + ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);
> + break;
> + }
> +
> ret = drm_atomic_crtc_set_property(crtc,
> crtc_state, prop, prop_value);
> break;
> @@ -1051,6 +1086,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
> case DRM_MODE_OBJECT_PLANE: {
> struct drm_plane *plane = obj_to_plane(obj);
> struct drm_plane_state *plane_state;
> + struct drm_mode_config *config = &plane->dev->mode_config;
>
> plane_state = drm_atomic_get_plane_state(state, plane);
> if (IS_ERR(plane_state)) {
> @@ -1058,6 +1094,13 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
> break;
> }
>
> + if (async_flip && prop != config->prop_fb_id) {
> + ret = drm_atomic_plane_get_property(plane, plane_state,
> + prop, &old_val);
> + ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);
> + break;
> + }
> +
> ret = drm_atomic_plane_set_property(plane,
> plane_state, file_priv,
> prop, prop_value);
> @@ -1349,6 +1392,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> struct drm_out_fence_state *fence_state;
> int ret = 0;
> unsigned int i, j, num_fences;
> + bool async_flip = false;
>
> /* disallow for drivers not supporting atomic: */
> if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
> @@ -1385,6 +1429,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> "commit failed: DRM_MODE_PAGE_FLIP_ASYNC not supported with atomic\n");
> return -EINVAL;
> }
> +
> + async_flip = true;
> }
>
> /* can't test and expect an event at the same time. */
> @@ -1469,8 +1515,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> goto out;
> }
>
> - ret = drm_atomic_set_property(state, file_priv,
> - obj, prop, prop_value);
> + ret = drm_atomic_set_property(state, file_priv, obj,
> + prop, prop_value, async_flip);
> if (ret) {
> drm_mode_object_put(obj);
> goto out;
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index 501a10edd0e1..381130cebe81 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -251,7 +251,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
> struct drm_file *file_priv,
> struct drm_mode_object *obj,
> struct drm_property *prop,
> - uint64_t prop_value);
> + uint64_t prop_value, bool async_flip);
> int drm_atomic_get_property(struct drm_mode_object *obj,
> struct drm_property *property, uint64_t *val);
>
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index ac0d2ce3f870..0e8355063eee 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -538,7 +538,7 @@ static int set_property_atomic(struct drm_mode_object *obj,
> obj_to_connector(obj),
> prop_value);
> } else {
> - ret = drm_atomic_set_property(state, file_priv, obj, prop, prop_value);
> + ret = drm_atomic_set_property(state, file_priv, obj, prop, prop_value, false);
> if (ret)
> goto out;
> ret = drm_atomic_commit(state);
> --
> 2.41.0
>
