[PATCH v3] drm: don't run atomic_async_check for disabled planes

[Murthy, Arun R]

On 05-08-2025 03:02, Xaver Hugl wrote:
> Am Mo., 4. Aug. 2025 um 11:54 Uhr schrieb Murthy, Arun R
> <arun.r.murthy at 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