Re: [PATCH wayland-protocols v5] unstable/drm-lease: DRM lease protocol support
Hi Marius, On Thu, 2018-09-06 at 11:29 +, Marius-cristian Vlad wrote: > > [...] > > > - removed 'revoked' event entirely as it adds complexity without > > > adding > > > too much benefit. > > > > The client will notice this via the leased drm fd sooner or later > > anyway, so it seems that this event is not strictly necessary. I > > wonder if there is any value in letting the client know immediately, > > though. For example, if a client displays mostly static content > > (like a presentation running on a non-desktop projector), it could > > be a long while until the next page flip attempt. [...] > > Currently, all heads without enabled output are skipped, and trying > > to lease a disabled monitor with: > > > > [output] > > name=HDMI-A-1 > > leasable=on > > mode=off > > > > crashes the compositor. > > Yes, I've never taken this into consideration. I assume this is the > case for HMDs where by default the connector will be disconnected? Yes, in fact if the current implementation [1] is accepted, it will by default behave exactly as if "mode=off" was set for non-desktop displays in weston.ini. [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/12 > The output contains the scanout_plane which contains the plane id, > and obviously for a disconnected output this will not be the case. I see, that is a problem. drm_output contains the crtc_id as well, so do we just need a way to attach a drm_head to a disabled drm_output? [...] > > What happens if a new connector becomes available, either because it > > is > > physically plugged in, or because another client cancels its lease? > > This could be handled by sending the connector advertisement (again), > > in which case leasable connectors could appear any time, not only > > upon > > connecting. > > Care to explain a bit how exactly does the client reaches that state? I was thinking first about a hypothetical presentation application that gets notified as soon as a projector is plugged into a non-desktop connector. That way one could start the presentation program and plug in the connector in either order and never have the desktop spill out onto the projector. [...] > > Not sure if the "connector_add_failed" and "connector_added" events > > are necessary. If something is wrong, the following "create" request > > can just return the "failed" event. > > I've kept these events because "failed" event seemed to generic for me, > and explained a little bit in the beginning another reason why I've > kept them. It would be great to add a note that these don't have to be waited for, i.e. that it is possible to just call: -> "add_connector" -> "add_connector" -> "create" without inserting round-trips to wait for these events in-between. regards Philipp ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols v5] unstable/drm-lease: DRM lease protocol support
[resending... maybe this time works] On Wed, 2018-09-05 at 08:55 +0200, Philipp Zabel wrote: > Hi Marius, > > thank you for the update! Thank you for taking the time to look over this. > > Am Dienstag, den 04.09.2018, 17:39 +0300 schrieb Marius Vlad: > > Simple protocol extension to manage DRM lease. Based on the work by > > Keith > > Packard in [1], respectively [2]. > > > > [1] https://emea01.safelinks.protection.outlook.com/?url=https%3A%2 > > F%2Fcgit.freedesktop.org%2Fmesa%2Fdrm%2Fcommit%2F%3Fid%3Dc417153538 > > 9d72e9135c9615cecd07b346fd6d7e&data=02%7C01%7Cmarius- > > cristian.vlad%40nxp.com%7C620b40c9cbc5430e8a4b08d612fc9d85%7C686ea1 > > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636717273515580055&sdata=p > > qLcIrKTlgSJ0fVyfdXOXO4Re%2BV6OrNQGccIhMSn0Os%3D&reserved=0 > > [2] https://emea01.safelinks.protection.outlook.com/?url=https%3A%2 > > F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2F > > linux.git%2Fcommit%2F%3Fh%3Dv4.15- > > rc9%26id%3D62884cd386b876638720ef88374b31a84ca7ee5f&data=02%7C0 > > 1%7Cmarius- > > cristian.vlad%40nxp.com%7C620b40c9cbc5430e8a4b08d612fc9d85%7C686ea1 > > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636717273515580055&sdata=A > > 3EfXGUUJMl%2FFQHw8LVHPn9Ptu%2BXKAr90EIwnHpLhzw%3D&reserved=0 > > > > Changes since v4: > > [...] > > - removed 'revoked' event entirely as it adds complexity without > > adding > > too much benefit. > > The client will notice this via the leased drm fd sooner or later > anyway, so it seems that this event is not strictly necessary. I > wonder > if there is any value in letting the client know immediately, though. > For example, if a client displays mostly static content (like a > presentation running on a non-desktop projector), it could be a long > while until the next page flip attempt. > > > Signed-off-by: Marius Vlad > > --- > > > > An implementation for this version can be found at > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > gitlab.freedesktop.org%2Fmarius.vlad0%2Fweston%2Fcommits%2Fdrm- > > lease-advertise-each-connnector-object- > > fixes&data=02%7C01%7Cmarius- > > cristian.vlad%40nxp.com%7C620b40c9cbc5430e8a4b08d612fc9d85%7C686ea1 > > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636717273515580055&sdata=M > > UrWEApwiGgSvcz8yKiT9Tm8Ij3UpGs6Ai5ioTtuWDI%3D&reserved=0 > > > > I tried the current weston implementation with this protocol version, > and I think the leased property and lease object have to be moved > from > output to head. > Currently, all heads without enabled output are skipped, and trying > to > lease a disabled monitor with: > > [output] > name=HDMI-A-1 > leasable=on > mode=off > > crashes the compositor. Yes, I've never taken this into consideration. I assume this is the case for HMDs where by default the connector will be disconnected? The output contains the scanout_plane which contains the plane id, and obviously for a disconnected output this will not be the case. I fixed the crash by checking if the output is set, but most likely this not the proper fix, if this is required. > > [...] > > + Upon connecting to the compositor all leasable connectors > > will be > > + advertised to the client. > > What happens if a client has received the full list of leasable > connectors and then one of those becomes unavailable, either because > it > is physically unplugged, or because it is leased to another client? > Is the client notified about this? > > What happens if a new connector becomes available, either because it > is > physically plugged in, or because another client cancels its lease? > This could be handled by sending the connector advertisement (again), > in which case leasable connectors could appear any time, not only > upon > connecting. Care to explain a bit how exactly does the client reaches that state? "connector_added", "connected_add_failed" are events that shrink the window in which the advertised connectors might be different when actually adding the connector. > > > These connectors are represented by > > + zwp_kms_lease_connector_v1 interface, and have to be "added" > > to a lease > > + request object before creating a lease object. This > > instructs the > > + compositor to use that connector when creating a lease. The > > client can > > + receive multiple events for multiple leasable connectors and > > needs a way > > + to discern between various leasable connectors. When > > receiving this > > + connector object, events will be sent gratuitously so that > > the > > + client can properly choose which connector wants to use. > > + > > + A lease request object is represented by > > zwp_kms_lease_request_v1 > > + interface and is used temporarily as a storage place for > > objects > > > s/objects/object/ It's a confusion of terminology. Objects in compositor and objects for the lease creation. > > > + IDs. Once the client has a lease object it can freely call > > its > > + de
Re: [PATCH wayland-protocols v5] unstable/drm-lease: DRM lease protocol support
___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
A keybinder module for Weston
Hello! A while ago I was messing around with Weston and implemented its biggest shortcoming: user keybindings. You can check out the simple module at https://github.com/tarvi-verro/weston-binder. It would be nice to have it merged with upstream, so don't hesitate to leave feedback to this end. Tarvi ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/3] compositor-drm: Add support for drm plane FB_DAMAGE_CLIPS property
On Wed, Sep 05, 2018 at 05:18:16PM -0700, Deepak Rawat wrote:
> The plane property FB_DAMAGE_CLIPS provides a way mark damaged regions
> on the plane in framebuffer coordinates of the framebuffer attached to
> the plane.
>
> This patch adds a new member "damage" to compositor version of
> drm_plane_state and set FB_DAMAGE_CLIPS property whenever damage is
> available.
>
> Signed-off-by: Deepak Rawat
> Cc: dri-de...@lists.freedesktop.org
Note that weston has switched over to gitlab based merge requests:
https://gitlab.freedesktop.org/wayland/weston/blob/master/CONTRIBUTING.md
Cheers, Daniel
> ---
> libweston/compositor-drm.c | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 6a87a296..30f6fce9 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -129,6 +129,7 @@ enum wdrm_plane_property {
> WDRM_PLANE_FB_ID,
> WDRM_PLANE_CRTC_ID,
> WDRM_PLANE_IN_FORMATS,
> + WDRM_PLANE_FB_DAMAGE_CLIPS,
> WDRM_PLANE__COUNT
> };
>
> @@ -171,6 +172,7 @@ static const struct drm_property_info plane_props[] = {
> [WDRM_PLANE_FB_ID] = { .name = "FB_ID", },
> [WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", },
> [WDRM_PLANE_IN_FORMATS] = { .name = "IN_FORMATS" },
> + [WDRM_PLANE_FB_DAMAGE_CLIPS] = { .name = "FB_DAMAGE_CLIPS" },
> };
>
> /**
> @@ -403,6 +405,8 @@ struct drm_plane_state {
>
> bool complete;
>
> + pixman_region32_t damage; /* damage sent to kernel */
> +
> struct wl_list link; /* drm_output_state::plane_list */
> };
>
> @@ -1306,6 +1310,7 @@ drm_plane_state_alloc(struct drm_output_state
> *state_output,
> assert(state);
> state->output_state = state_output;
> state->plane = plane;
> + pixman_region32_init(&state->damage);
>
> /* Here we only add the plane state to the desired link, and not
>* set the member. Having an output pointer set means that the
> @@ -1336,6 +1341,7 @@ drm_plane_state_free(struct drm_plane_state *state,
> bool force)
> wl_list_remove(&state->link);
> wl_list_init(&state->link);
> state->output_state = NULL;
> + pixman_region32_fini(&state->damage);
>
> if (force || state != state->plane->state_cur) {
> drm_fb_unref(state->fb);
> @@ -1373,6 +1379,7 @@ drm_plane_state_duplicate(struct drm_output_state
> *state_output,
> if (src->fb)
> dst->fb = drm_fb_ref(src->fb);
> dst->output_state = state_output;
> + pixman_region32_init(&dst->damage);
> dst->complete = false;
>
> return dst;
> @@ -2409,6 +2416,33 @@ drm_mode_ensure_blob(struct drm_backend *backend,
> struct drm_mode *mode)
> return ret;
> }
>
> +static int
> +plane_add_damage(drmModeAtomicReq *req, struct drm_backend *backend,
> + struct drm_plane_state *plane_state)
> +{
> + struct drm_plane *plane = plane_state->plane;
> + pixman_box32_t *rects;
> + uint32_t blob_id;
> + int n_rects;
> + int ret;
> +
> + if (!pixman_region32_not_empty(&plane_state->damage))
> + return 0;
> +
> + rects = pixman_region32_rectangles(&plane_state->damage, &n_rects);
> +
> + ret = drmModeCreatePropertyBlob(backend->drm.fd, rects,
> + sizeof(*rects) * n_rects, &blob_id);
> + if (ret != 0)
> + return ret;
> +
> + ret = plane_add_prop(req, plane, WDRM_PLANE_FB_DAMAGE_CLIPS, blob_id);
> + if (ret != 0)
> + return ret;
> +
> + return 0;
> +}
> +
> static int
> drm_output_apply_state_atomic(struct drm_output_state *state,
> drmModeAtomicReq *req,
> @@ -2477,6 +2511,7 @@ drm_output_apply_state_atomic(struct drm_output_state
> *state,
> plane_state->dest_w);
> ret |= plane_add_prop(req, plane, WDRM_PLANE_CRTC_H,
> plane_state->dest_h);
> + ret |= plane_add_damage(req, backend, plane_state);
>
> if (ret != 0) {
> weston_log("couldn't set plane state\n");
> --
> 2.17.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
