On Tue, 26 Sep 2017 18:15:48 +0100 Daniel Stone <dani...@collabora.com> wrote:
> Add support for using the atomic-modesetting API to apply output state.
> Unlike previous series, this commit does not unflip sprites_are_broken,
> until further work has been done with assign_planes to make it reliable.
>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> Co-authored-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> Co-authored-by: Louis-Francis Ratté-Boulianne
> <louis-francis.ratte-boulia...@collabora.com>
> Co-authored-by: Derek Foreman <derek.fore...@collabora.co.uk>
> ---
> configure.ac | 2 +-
> libweston/compositor-drm.c | 507
> ++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 434 insertions(+), 75 deletions(-)
Hi Daniel!
> +#ifdef HAVE_DRM_ATOMIC
> +static int
> +crtc_add_prop(drmModeAtomicReq *req, struct drm_output *output,
> + enum wdrm_crtc_property prop, uint64_t val)
> +{
> + struct drm_property_info *info = &output->props_crtc[prop];
> + int ret;
> +
> + if (!info)
> + return -1;
Should not this and the two functions below check that the prop_id was
actually found?
What happens if one uses property ID 0?
> +
> + ret = drmModeAtomicAddProperty(req, output->crtc_id, info->prop_id,
> + val);
> + return (ret <= 0) ? -1 : 0;
> +}
> +
> +static int
> +connector_add_prop(drmModeAtomicReq *req, struct drm_output *output,
> + enum wdrm_connector_property prop, uint64_t val)
> +{
> + struct drm_property_info *info = &output->props_conn[prop];
> + int ret;
> +
> + if (!info)
> + return -1;
> +
> + ret = drmModeAtomicAddProperty(req, output->connector_id,
> + info->prop_id, val);
> + return (ret <= 0) ? -1 : 0;
> +}
> +
> +static int
> +plane_add_prop(drmModeAtomicReq *req, struct drm_plane *plane,
> + enum wdrm_plane_property prop, uint64_t val)
> +{
> + struct drm_property_info *info = &plane->props[prop];
> + int ret;
> +
> + if (!info)
> + return -1;
> +
> + ret = drmModeAtomicAddProperty(req, plane->plane_id, info->prop_id,
> + val);
> + return (ret <= 0) ? -1 : 0;
> +}
> +static int
> +drm_pending_state_apply_atomic(struct drm_pending_state *pending_state)
> +{
> + struct drm_backend *b = pending_state->backend;
> + struct drm_output_state *output_state, *tmp;
> + drmModeAtomicReq *req = drmModeAtomicAlloc();
> + uint32_t flags = 0;
> + int ret = 0;
> +
> + if (!req)
> + return -1;
> +
> + if (b->state_invalid) {
> + uint32_t *unused;
> + int err;
> +
> + /* If we need to reset all our state (e.g. because we've
> + * just started, or just been VT-switched in), explicitly
> + * disable all the CRTCs and connectors we aren't using. */
> + wl_array_for_each(unused, &b->unused_connectors) {
> + struct drm_property_info infos[WDRM_CONNECTOR__COUNT];
> + struct drm_property_info *info;
> + drmModeObjectProperties *props;
> +
> + memset(infos, 0, sizeof(infos));
memset would be easier done with
struct drm_property_info infos[WDRM_CONNECTOR__COUNT] = {};
> +
> + props = drmModeObjectGetProperties(b->drm.fd,
> + *unused,
> +
> DRM_MODE_OBJECT_CONNECTOR);
> + if (!props) {
> + ret = -1;
> + continue;
> + }
> +
> + drm_property_info_populate(b, connector_props, infos,
> + WDRM_CONNECTOR__COUNT,
> + props);
> + drmModeFreeObjectProperties(props);
> +
> + info = &infos[WDRM_CONNECTOR_CRTC_ID];
> + err = drmModeAtomicAddProperty(req, *unused,
> + info->prop_id, 0);
> + if (err <= 0)
> + ret = -1;
> +
> + info = &infos[WDRM_CONNECTOR_DPMS];
> + if (info->prop_id > 0)
> + err = drmModeAtomicAddProperty(req, *unused,
> + info->prop_id,
> +
> DRM_MODE_DPMS_OFF);
> + if (err <= 0)
> + ret = -1;
> +
> + drm_property_info_free(infos, WDRM_CONNECTOR__COUNT);
> + }
> +
> + wl_array_for_each(unused, &b->unused_crtcs) {
> + struct drm_property_info infos[WDRM_CRTC__COUNT];
> + struct drm_property_info *info;
> + drmModeObjectProperties *props;
> + uint64_t active;
> +
> + memset(infos, 0, sizeof(infos));
> +
> + /* We can't emit a disable on a CRTC that's already
> + * off, as the kernel will refuse to generate an event
> + * for an off->off state and fail the commit.
> + */
> + props = drmModeObjectGetProperties(b->drm.fd,
> + *unused,
> +
> DRM_MODE_OBJECT_CRTC);
> + if (!props) {
> + ret = -1;
> + continue;
> + }
> +
> + drm_property_info_populate(b, crtc_props, infos,
> + WDRM_CRTC__COUNT,
> + props);
> +
> + info = &infos[WDRM_CRTC_ACTIVE];
> + active = drm_property_get_value(info, props, 0);
> + drmModeFreeObjectProperties(props);
> + if (active == 0) {
> + drm_property_info_free(infos, WDRM_CRTC__COUNT);
> + continue;
> + }
> +
> + err = drmModeAtomicAddProperty(req, *unused,
> + info->prop_id, 0);
> + if (err <= 0)
> + ret = -1;
> +
> + info = &infos[WDRM_CRTC_MODE_ID];
> + err = drmModeAtomicAddProperty(req, *unused,
> + info->prop_id, 0);
> + if (err <= 0)
> + ret = -1;
> +
> + drm_property_info_free(infos, WDRM_CRTC__COUNT);
> + }
> +
> +#if 0
> + wl_array_for_each(unused, &b->unused_planes) {
I don't think we would ever need a unused_planes array since we create
a drm_plane for them all, right?
Should just go through the list of drm_planes.
> + struct drm_property_info infos[WDRM_PLANE__COUNT];
> + struct drm_property_info *info;
> + drmModeObjectProperties *props;
> +
> + memset(infos, 0, sizeof(infos));
> +
> + drm_property_info_populate(b, props_plane, infos,
> + WDRM_PLANE__COUNT, props);
> + drmModeFreeObjectProperties(props);
> +
> + info = &infos[WDRM_PLANE_CRTC_ID];
> + err = drmModeAtomicAddProperty(req, *unused,
> + info->prop_id, 0);
> + if (err <= 0)
> + ret = -1;
> +
> + info = &infos[WDRM_PLANE_FB_ID];
> + err = drmModeAtomicAddProperty(req, *unused,
> + info->prop_id, 0);
> + if (err <= 0)
> + ret = -1;
> +
> + drm_property_info_free(infos, WDRM_PLANE__COUNT);
> + }
> +#endif
> +
> + flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
> + }
> +
> + wl_list_for_each(output_state, &pending_state->output_list, link)
> + ret |= drm_output_apply_state_atomic(output_state, req, &flags);
> +
> + if (ret != 0) {
> + weston_log("atomic: couldn't compile atomic state\n");
> + goto out;
> + }
> +
> + flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
> + ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
> + if (ret != 0) {
> + weston_log("atomic: couldn't commit new state: %m\n");
> + goto out;
> + }
> +
> + wl_list_for_each_safe(output_state, tmp, &pending_state->output_list,
> + link) {
> + drm_output_assign_state(output_state,
> + DRM_OUTPUT_STATE_UPDATE_ASYNCHRONOUS);
> + }
> +
> + b->state_invalid = false;
> +
> + assert(wl_list_empty(&pending_state->output_list));
> +
> +out:
> + drmModeAtomicFree(req);
Missing drm_pending_state_free()?
> + return ret;
> +}
> +#endif
> +
> static int
> drm_pending_state_apply(struct drm_pending_state *pending_state)
> {
Otherwise looks good to me.
Thanks,
pq
pgpfwWFEP4YBU.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
