On 05-08-2025 03:02, Xaver Hugl wrote:
Am Mo., 4. Aug. 2025 um 11:54 Uhr schrieb Murthy, Arun R
<arun.r.mur...@intel.com>:
On 01-08-2025 18:40, Xaver Hugl wrote:
It's entirely valid and correct for compositors to include disabled
planes in the atomic commit, and doing that should not prevent async
flips from working. To fix that, this commit moves the plane check
to after all the properties of the object have been set,
I dont think this is required. Again the plane states will have to be
fetched outside the set_prop()
Alternate approach
@@ -1091,8 +1091,16 @@ int drm_atomic_set_property(struct
drm_atomic_state *state,
/* ask the driver if this non-primary plane is
supported */
if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
- ret = -EINVAL;
+ /*
+ * continue if no change in prop on
non-supported async planes as well
+ * or when disabling the plane
+ */
+ if (ret == 0 || (prop ==
config->prop_fb_id && prop_value == 0))
This would allow disabling a plane in an async commit that was
previously enabled, not sure that should be allowed?
Yes, I assumes this is what you intended to do as you had if
(curr_state->visible or prev_state->visible) in your 1st patchset changes.
If disabling of plane is not suppose to be allowed then prop_value == 0
condition should be removed.
Also, if the
property is fb_id, ret would be used uninitialized. But you're right,
this should be fixable with smaller changes. Probably best to keep it
minimal for the bugfix.
Looking more at this code, I also notice that it currently allows you
to change *any* property on overlay planes in an async flip, which
doesn't seem right.
As part of ret = drm_atomci_check_prop_changes() ret will be -EINVAL.
ret should be checked for failure and returned over here.
Thanks and Regards,
Arun R Murthy
--------------------
+ drm_dbg_atomic(prop->dev,
+ "[PLANE:%d:%s] continue async as there is no prop change\n",
+ obj->id,
plane->name);
+ else
+ ret = -EINVAL;
if (plane_funcs &&
plane_funcs->atomic_async_check)
Thanks and Regards,
Arun R Murthy
