On Wed, 20 Dec 2017 12:26:27 +0000 Daniel Stone <dani...@collabora.com> wrote:
> If we have an unused CRTC or connector, explicitly disable it during the
> end of the repaint cycle, or when we get VT-switched back in.
>
> This commit moves state_invalid from an output property to a backend
> property, as the unused CRTCs or connectors are likely not tracked by
> drm_outputs. This matches the mechanics of later commits, where we move
> to a global repaint-flush hook, applying the state for all outputs in
> one go.
Hi Daniel,
I have a recollection of having a rationale that the state_invalid flag
should stay per-output, but it seems those ideas were just optimization
that's probably moot anyway. I suppose with atomic, the per-output flag
would not make any difference to a global flag, since the atomic commit
would be modeset-or-not as a whole?
I wonder, could the following happen:
- have outputs A and B at different timing phase such that they usually
don't coalesce to the same atomic update
- output A needs a modeset
- output B repaints first, does modeset, and clears the flag
- output A does not get a modeset
Would we actually need per-output flags and a flag for the unused CRTCs?
Clone mode will bring new causes for "needs a modeset": attaching or
detaching a head from a running output.
If we were good with just a global flag, then this patch would be good.
Thanks,
pq
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> ---
> libweston/compositor-drm.c | 35 +++++++++++++++++++++--------------
> 1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 9c7564383..87c2603c7 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -188,6 +188,7 @@ struct drm_backend {
>
> void *repaint_data;
>
> + bool state_invalid;
> struct wl_array unused_connectors;
> struct wl_array unused_crtcs;
>
> @@ -350,8 +351,6 @@ struct drm_output {
> enum dpms_enum dpms;
> struct backlight *backlight;
>
> - bool state_invalid;
> -
> int vblank_pending;
> int page_flip_pending;
> int destroy_pending;
> @@ -1748,7 +1747,7 @@ drm_output_repaint(struct weston_output *output_base,
> assert(scanout_state->dest_h == scanout_state->src_h >> 16);
>
> mode = container_of(output->base.current_mode, struct drm_mode, base);
> - if (output->state_invalid || !scanout_plane->state_cur->fb ||
> + if (backend->state_invalid || !scanout_plane->state_cur->fb ||
> scanout_plane->state_cur->fb->stride != scanout_state->fb->stride) {
> ret = drmModeSetCrtc(backend->drm.fd, output->crtc_id,
> scanout_state->fb->fb_id,
> @@ -1760,9 +1759,7 @@ drm_output_repaint(struct weston_output *output_base,
> goto err;
> }
> output_base->set_dpms(output_base, WESTON_DPMS_ON);
> -
> - output->state_invalid = false;
> - }
> + }
>
> if (drmModePageFlip(backend->drm.fd, output->crtc_id,
> scanout_state->fb->fb_id,
> @@ -1869,7 +1866,7 @@ drm_output_start_repaint_loop(struct weston_output
> *output_base)
> /* Need to smash all state in from scratch; current timings might not
> * be what we want, page flip might not work, etc.
> */
> - if (output->state_invalid)
> + if (backend->state_invalid)
> goto finish_frame;
>
> assert(scanout_plane->state_cur->output == output);
> @@ -2033,12 +2030,26 @@ drm_repaint_flush(struct weston_compositor
> *compositor, void *repaint_data)
> struct drm_backend *b = to_drm_backend(compositor);
> struct drm_pending_state *pending_state = repaint_data;
> struct drm_output_state *output_state, *tmp;
> + uint32_t *unused;
> +
> + if (b->state_invalid) {
> + /* 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 we aren't using. This also disables
> + * all connectors on these CRTCs, so we don't need to do that
> + * separately with the pre-atomic API. */
> + wl_array_for_each(unused, &b->unused_crtcs)
> + drmModeSetCrtc(b->drm.fd, *unused, 0, 0, 0, NULL, 0,
> + NULL);
> + }
>
> wl_list_for_each_safe(output_state, tmp, &pending_state->output_list,
> link) {
> drm_output_assign_state(output_state, DRM_STATE_APPLY_ASYNC);
> }
>
> + b->state_invalid = false;
> +
> drm_pending_state_free(pending_state);
> b->repaint_data = NULL;
> }
> @@ -2659,7 +2670,7 @@ drm_output_switch_mode(struct weston_output
> *output_base, struct weston_mode *mo
> * sledgehammer modeswitch first, and only later showing new
> * content.
> */
> - output->state_invalid = true;
> + b->state_invalid = true;
>
> if (b->use_pixman) {
> drm_output_fini_pixman(output);
> @@ -4000,8 +4011,6 @@ drm_output_enable(struct weston_output *base)
> output->connector->count_modes == 0 ?
> ", built-in" : "");
>
> - output->state_invalid = true;
> -
> return 0;
>
> err:
> @@ -4516,10 +4525,7 @@ session_notify(struct wl_listener *listener, void
> *data)
> weston_log("activating session\n");
> weston_compositor_wake(compositor);
> weston_compositor_damage_all(compositor);
> -
> - wl_list_for_each(output, &compositor->output_list, base.link)
> - output->state_invalid = true;
> -
> + b->state_invalid = true;
> udev_input_enable(&b->input);
> } else {
> weston_log("deactivating session\n");
> @@ -4931,6 +4937,7 @@ drm_backend_create(struct weston_compositor *compositor,
> if (b == NULL)
> return NULL;
>
> + b->state_invalid = true;
> b->drm.fd = -1;
> wl_array_init(&b->unused_crtcs);
> wl_array_init(&b->unused_connectors);
pgp_TngSAlnUy.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
