Re: wl_surface::attach(NULL) release previous buffer?

[Vlad Zahorodnii]

On 9/14/23 14:24, John Cox wrote:


Hi

A, hopefully, simple question - should I expect a wl_buffer::release
event from the buffer previously committed to a surface after I've
attached (and invalidated & committed) a NULL buffer to the surface? it
doesn't seem to happen for me (I have WAYLAND_DEBUG=1 logs showing it
not happening).

If I shouldn't expect a release - when is it safe to reuse/free the
buffer storage?


The compositor may continue using the buffer even if you attach a null 
buffer to the wl_surface. For example, the compositor may do it to play 
an animation if the window is unmapped.


It is safe to reuse the buffer when you receive wl_buffer.release event 
from the compositor.


Regards,
Vlad

Re: wl_surface::attach(NULL) release previous buffer?

[John Cox]

Hi

>On 9/14/23 14:24, John Cox wrote:
>
>> Hi
>>
>> A, hopefully, simple question - should I expect a wl_buffer::release
>> event from the buffer previously committed to a surface after I've
>> attached (and invalidated & committed) a NULL buffer to the surface? it
>> doesn't seem to happen for me (I have WAYLAND_DEBUG=1 logs showing it
>> not happening).
>>
>> If I shouldn't expect a release - when is it safe to reuse/free the
>> buffer storage?
>
>The compositor may continue using the buffer even if you attach a null 
>buffer to the wl_surface. For example, the compositor may do it to play 
>an animation if the window is unmapped.

You've completely lost me with this example.  Are you suggesting that
the compositor is reusing my buffer for its own purposes?

>It is safe to reuse the buffer when you receive wl_buffer.release event 
>from the compositor.

Yup I got that (and you also have to wait for dmabuf fences if it is
that sort of buffer).

In the case that I was wondering about it was a compositor bug that kept
the buffer locked when it shouldn't have been.

Thanks

HJC

Re: [Freedreno] [PATCH RFC v6 07/10] drm/atomic: Loosen FB atomic checks

[Jessica Zhang]

On 8/29/2023 1:22 AM, Pekka Paalanen wrote:

On Mon, 28 Aug 2023 17:05:13 -0700
Jessica Zhang  wrote:


Loosen the requirements for atomic and legacy commit so that, in cases
where pixel_source != FB, the commit can still go through.

This includes adding framebuffer NULL checks in other areas to account for
FB being NULL when non-FB pixel sources are enabled.

To disable a plane, the pixel_source must be NONE or the FB must be NULL
if pixel_source == FB.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic.c| 20 +++-
  drivers/gpu/drm/drm_atomic_helper.c | 36 
  include/drm/drm_atomic_helper.h |  4 ++--
  include/drm/drm_plane.h | 29 +
  4 files changed, 62 insertions(+), 27 deletions(-)


...


diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index a58f84b6bd5e..4c5b7bcdb25c 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -992,6 +992,35 @@ static inline struct drm_plane *drm_plane_find(struct 
drm_device *dev,
  #define drm_for_each_plane(plane, dev) \
list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
  
+/**

+ * drm_plane_solid_fill_enabled - Check if solid fill is enabled on plane
+ * @state: plane state
+ *
+ * Returns:
+ * Whether the plane has been assigned a solid_fill_blob
+ */
+static inline bool drm_plane_solid_fill_enabled(struct drm_plane_state *state)
+{
+   if (!state)
+   return false;
+   return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_SOLID_FILL && 
state->solid_fill_blob;
+}
+
+static inline bool drm_plane_has_visible_data(const struct drm_plane_state 
*state)
+{
+   switch (state->pixel_source) {
+   case DRM_PLANE_PIXEL_SOURCE_NONE:
+   return false;
+   case DRM_PLANE_PIXEL_SOURCE_SOLID_FILL:
+   return state->solid_fill_blob != NULL;


This reminds me, new UAPI docs did not say what the requirements are for
choosing solid fill pixel source. Is the atomic commit rejected if
pixel source is solid fill, but solid_fill property has no blob?


Hi Pekka,

Yes, if pixel_source is solid_fill and the solid_fill property blob 
isn't set, the atomic commit should throw an error.


Will document this in the UAPI.

Thanks,

Jessica Zhang



This should be doc'd.


Thanks,
pq


+   case DRM_PLANE_PIXEL_SOURCE_FB:
+   default:
+   WARN_ON(state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB);
+   }
+
+   return state->fb != NULL;
+}
+
  bool drm_any_plane_has_format(struct drm_device *dev,
  u32 format, u64 modifier);