Re: xfwm4-wayland: wlroots branch merged into wayland branch
My wayland port of xfwm4 currently consists of two binaries, a compositor and a helper client. The compositor sends events to the helper client using a private protocol. The helper client controls the compositor. The helper client basically contains all the code of the original xfwm4. The tabwin widget is rendered by the helper client. Essentially my Wayland compositor is behaving like an X server and my helper client acts like a traditional X11 window manager. The compositor-and-helper-client approach works fine for the tabwin, but it may have limitations when attempting to port server-side decorations. I suppose copying the X11 approach could result in a lot of pointless back-and-forth between the helper client and server. How could I port xfwm4's window decorations code to Wayland? Also, window thumbnails may be a very difficult problem. There isn't even an agreed way to do that on Wayland. Should the compositor render the thumbnails, or should it pass the window's buffer to a client? Or another way I haven't thought of? My compositor currently consists of two binaries. I suppose if I wanted to make it one binary, the GTK code would have to be removed as afaik you can't do GTK server side? I don't necessarily want to make it one binary. How does mutter manage to be one binary? On X11, the window manager is a client but on Wayland the window manager is a server, so the directionality of X11 and Wayland is the complete opposite. Regards adlo > On 16 Dec 2022, at 06:58, adlo wrote: > > The wlroots version of xfwm4-wayland now has partial support for the > wlr-foreign-toplevel protocol. > > I have now declared feature parity with the libweston version, so I > have merged the wlroots branch into the wayland branch. > > Link to the git repository: > https://github.com/adlocode/xfwm4/tree/wayland > > Regards > adlo
Re: [RFC PATCH v2 2/3] drm: Adjust atomic checks for solid fill color
On 23/12/2022 00:14, Jessica Zhang wrote:
Loosen the requirements for atomic and legacy commit so that, in cases
where solid fill planes is enabled (and FB_ID is NULL), the commit can
still go through.
In addition, add framebuffer NULL checks in other areas to account for
FB being NULL when solid fill is enabled.
Changes in V2:
- Changed to checks for if solid_fill_blob is set (Dmitry)
- Abstracted (plane_state && !solid_fill_blob) checks to helper method
(Dmitry)
- Fixed indentation issue (Dmitry)
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/drm_atomic.c| 69 -
drivers/gpu/drm/drm_atomic_helper.c | 34 --
drivers/gpu/drm/drm_plane.c | 8 ++--
include/drm/drm_atomic_helper.h | 6 ++-
include/drm/drm_plane.h | 12 +
5 files changed, 78 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f197f59f6d99..b92d75bda7fd 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -601,8 +601,10 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
uint32_t num_clips;
int ret;
- /* either *both* CRTC and FB must be set, or neither */
- if (crtc && !fb) {
+ /* When solid_fill is disabled,
+* either *both* CRTC and FB must be set, or neither
+*/
The cause and effect logic seems to be broken here. I'd expect something
like: "When CRTC is set, at least one of fb and solid_fill should be set."
Also it might be worth defining
bool drm_plane_has_visible_data(struct drm_plane_state *state)
{
return state->fb || drm_plane_solid_fill_enabled(state);
}
Then you can use this function here and below, where you check both fb
and solid_fill property.
+ if (crtc && !fb && !new_plane_state->solid_fill_blob) {
drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no FB\n",
plane->base.id, plane->name);
return -EINVAL;
@@ -626,14 +628,17 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
}
/* Check whether this plane supports the fb pixel format. */
- ret = drm_plane_check_pixel_format(plane, fb->format->format,
- fb->modifier);
- if (ret) {
- drm_dbg_atomic(plane->dev,
- "[PLANE:%d:%s] invalid pixel format %p4cc, modifier
0x%llx\n",
- plane->base.id, plane->name,
- &fb->format->format, fb->modifier);
- return ret;
+ if (fb) {
+ ret = drm_plane_check_pixel_format(plane, fb->format->format,
+ fb->modifier);
+
+ if (ret) {
+ drm_dbg_atomic(plane->dev,
+ "[PLANE:%d:%s] invalid pixel format %p4cc,
modifier 0x%llx\n",
+ plane->base.id, plane->name,
+ &fb->format->format, fb->modifier);
+ return ret;
+ }
I'd suggest merging twof if(fb) blocks and extracting them to a separate
functions.
}
/* Give drivers some help against integer overflows */
@@ -649,28 +654,30 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
return -ERANGE;
}
- fb_width = fb->width << 16;
- fb_height = fb->height << 16;
+ if (fb) {
+ fb_width = fb->width << 16;
+ fb_height = fb->height << 16;
- /* Make sure source coordinates are inside the fb. */
- if (new_plane_state->src_w > fb_width ||
- new_plane_state->src_x > fb_width - new_plane_state->src_w ||
- new_plane_state->src_h > fb_height ||
- new_plane_state->src_y > fb_height - new_plane_state->src_h) {
- drm_dbg_atomic(plane->dev,
- "[PLANE:%d:%s] invalid source coordinates "
- "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
- plane->base.id, plane->name,
- new_plane_state->src_w >> 16,
- ((new_plane_state->src_w & 0x) * 15625) >>
10,
- new_plane_state->src_h >> 16,
- ((new_plane_state->src_h & 0x) * 15625) >>
10,
- new_plane_state->src_x >> 16,
- ((new_plane_state->src_x & 0x) * 15625) >>
10,
- new_plane_state->src_y >> 16,
- ((new_plane_state->src_y & 0x) * 15625) >>
10,
- fb->width, fb->height);
- return -ENOSPC;
+ /* Make sure source coordinates are inside t
Re: [RFC PATCH v2 3/3] drm/msm/dpu: Use color_fill property for DPU planes
On 23/12/2022 00:14, Jessica Zhang wrote:
Initialize and use the color_fill properties for planes in DPU driver. In
addition, relax framebuffer requirements within atomic commit path and
add checks for NULL framebuffers. Finally, drop DPU_PLANE_COLOR_FILL_FLAG
as it's unused.
Changes since V2:
- Fixed dropped 'const' warning
- Dropped use of solid_fill_format
- Switched to using drm_plane_solid_fill_enabled helper method
- Added helper to convert color fill to BGR888 (Rob)
- Added support for solid fill on planes of varying sizes
- Removed DPU_PLANE_COLOR_FILL_FLAG
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 ++-
2 files changed, 49 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 13ce321283ff..0695b70ea1b7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -409,6 +409,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
struct drm_plane_state *state;
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
struct dpu_plane_state *pstate = NULL;
+ const struct msm_format *fmt;
struct dpu_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
@@ -441,7 +442,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
sspp_idx - SSPP_VIG0,
state->fb ? state->fb->base.id : -1);
- format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
+ if (pstate->base.fb)
+ fmt = msm_framebuffer_format(pstate->base.fb);
+ else
+ fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base,
+ DRM_FORMAT_ABGR, 0);
+
+ format = to_dpu_format(fmt);
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 86719020afe2..51a7507373f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -44,7 +44,6 @@
#define DPU_NAME_SIZE 12
-#define DPU_PLANE_COLOR_FILL_FLAG BIT(31)
#define DPU_ZPOS_MAX 255
/* multirect rect index */
@@ -105,7 +104,6 @@ struct dpu_plane {
enum dpu_sspp pipe;
struct dpu_hw_pipe *pipe_hw;
- uint32_t color_fill;
bool is_error;
bool is_rt_pipe;
const struct dpu_mdss_cfg *catalog;
@@ -678,6 +676,17 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu,
&scaler3_cfg);
}
+static uint32_t _dpu_plane_get_fill_color(struct drm_solid_fill solid_fill)
+{
+ uint32_t ret = 0;
+
+ ret |= ((uint8_t) solid_fill.b) << 16;
+ ret |= ((uint8_t) solid_fill.g) << 8;
+ ret |= ((uint8_t) solid_fill.r);
+
+ return ret;
+}
+
/**
* _dpu_plane_color_fill - enables color fill on plane
* @pdpu: Pointer to DPU plane object
@@ -1001,12 +1010,17 @@ static int dpu_plane_atomic_check(struct drm_plane
*plane,
dst = drm_plane_state_dest(new_plane_state);
- fb_rect.x2 = new_plane_state->fb->width;
- fb_rect.y2 = new_plane_state->fb->height;
+ if (new_plane_state->fb) {
+ fb_rect.x2 = new_plane_state->fb->width;
+ fb_rect.y2 = new_plane_state->fb->height;
+ }
max_linewidth = pdpu->catalog->caps->max_linewidth;
- fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+ if (new_plane_state->fb)
+ fmt =
to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+ else
+ fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
@@ -1018,7 +1032,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
/* check src bounds */
- } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) {
+ } else if (new_plane_state->fb && !dpu_plane_validate_src(&src,
&fb_rect, min_src_size)) {
DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
DRM_RECT_ARG(&src));
return -E2BIG;
@@ -1086,9 +1100,10 @@ void dpu_plane_flush(struct drm_plane *plane)
if (pdpu->is_error)
/* force white frame with 100% alpha pipe output on error */
_dpu_plane_color_fill(pdpu, 0xFF, 0xFF);
- else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
+ else if (!(plane->state->fb) &&
drm_plane_solid_fill_enabled(plane->state))
And what if the plane has both fb and solid_fill proprety?
/* force 100% alpha */
- _dpu_plane_col
Re: xfwm4-wayland: wlroots branch merged into wayland branch
Hello there, > The compositor-and-helper-client approach works fine for the tabwin, but it > may have limitations when attempting to port server-side decorations. I > suppose copying the X11 approach could result in a lot of pointless > back-and-forth between the helper client and server. How could I port xfwm4's > window decorations code to Wayland? You can render that in the compositor, thats what sway does. > Also, window thumbnails may be a very difficult problem. There isn't even an > agreed way to do that on Wayland. Should the compositor render the > thumbnails, or should it pass the window's buffer to a client? Or another way > I haven't thought of? There's the foreign-toplevel-managment [1] protocol that could help you achieve that. It's based on the wlr-foreign-toplevel-managment-unstable-v1 [2] protocol from wlr-protocols, wlroots has support for it already. > My compositor currently consists of two binaries. I suppose if I wanted to > make it one binary, the GTK code would have to be removed as afaik you can't > do GTK server side? I don't necessarily want to make it one binary. Usually compositors do a server and a helper binary that sends commands over an IPC socket, like swaymsg [3] does. The messages a proper to the compositor. What stops you to do GTK on the server side? Do you need input from the user? Cheers, Simon [1]: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/156#note_1697279 [2]: https://gitlab.freedesktop.org/wlroots/wlr-protocols/-/blob/master/unstable/wlr-foreign-toplevel-management-unstable-v1.xml [3]: https://github.com/swaywm/sway/blob/master/swaymsg/swaymsg.1.scd
