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

[Kumar, Naveen1]

Hi Xaver,

>-----Original Message-----
>From: Xaver Hugl <xaver.hugl at kde.org>
>Sent: Tuesday, August 5, 2025 3:03 AM
>To: Murthy, Arun R <arun.r.murthy at intel.com>
>Cc: dri-devel at lists.freedesktop.org; andrealmeid at igalia.com;
>chris at kode54.net; Kumar, Naveen1 <naveen1.kumar at intel.com>;
>ville.syrjala at linux.intel.com; mdaenzer at redhat.com; intel-
>gfx at lists.freedesktop.org; amd-gfx at lists.freedesktop.org;
>alexdeucher at gmail.com
>Subject: Re: [PATCH v3] drm: don't run atomic_async_check for disabled planes
>
>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? 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.

As I commented earlier in the gitlab issue [1], any change of property, including disabling a plane is not allowed in the async commit. 
We must disable a plane (e.g. HW cursor) during the first (synchronized) flip, and allowing later flips to proceed asynchronously. 
This change should be done in the compositor. As per Ville's opinion in related series [2], kernel driver should reject all these disabled
Planes in the drm core and driver should only get the planes which is supported with async flip. Based on his comment, I have started
Working and will be addressing it in the next version of my series [3].
	
[1]. https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13834#note_2994595 
[2]. https://lore.kernel.org/all/aHAg2J-uFLLWINqp@intel.com/
[3]. https://patchwork.freedesktop.org/series/151280/

Regards,
Naveen Kumar
>
>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.
>
>> +  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