[PATCH 2/9] compositor: Output repaint in clone mode

Pekka Paalanen ppaalanen at gmail.com
Thu Feb 13 23:49:15 PST 2014


On Fri, 14 Feb 2014 15:17:37 +0800
Xiong Zhang <xiong.y.zhang at intel.com> wrote:

> Because slave output doesn't in compositor->output_list,
> the output->repaint()is called from master output only.
> When master output repaint,all the slave output should
> repaint also.
> 
> Slave output share fb with master output,when both slave and
> master have finished page flip, the fb obj can be released.

Hi,

ok, so core's finish_frame will be called when all flips of a master
output and its slaves have completed. If the refresh rates of these
outputs are different, I think it means that the perceived frame rate
(the frequency at which finish_frame is called) will fluctuate over
time. That will have very interesting consequences for presentation
prediction and feedback.

OTOH, the future will bring dynamically varied refresh rate monitors,
so client's should be prepared for fluctuations in the apparent vblank
frequency anyway.

I'm not claiming your patch creates a problem, I think we can cope with
it, but it will be interesting nevertheless. Just pointing out we
should keep this in mind.

We may need to tweak finish_frame calling logic a bit, so that it gets
the flip completion timestamp of the master output, and not whatever
output was the last one. But that will only be a concern when the
presentation extension lands. I think... unless we care about the
timestamp carried by the frame callbacks.


Thanks,
pq


> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>
> ---
>  src/compositor-drm.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/compositor.c     |   3 +-
>  2 files changed, 112 insertions(+), 3 deletions(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 6773226..ce85aec 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -155,6 +155,7 @@ struct drm_output {
>  
>  	uint32_t transform;
>  	int scale, mm_width, mm_height;
> +	uint32_t page_flip_count;
>  
>  	struct gbm_surface *surface;
>  	struct gbm_bo *cursor_bo[2];
> @@ -591,8 +592,9 @@ drm_output_repaint(struct weston_output *output_base,
>  	struct drm_sprite *s;
>  	struct drm_mode *mode;
>  	int ret = 0;
> +	struct drm_output *clone_output;
>  
> -	if (output->destroy_pending)
> +	if (output->destroy_pending || output->base.is_slave)
>  		return -1;
>  
>  	if (!output->next)
> @@ -612,6 +614,25 @@ drm_output_repaint(struct weston_output *output_base,
>  			goto err_pageflip;
>  		}
>  		output_base->set_dpms(output_base, WESTON_DPMS_ON);
> +
> +		wl_list_for_each(clone_output, &output->base.clone_output_list,
> +				 base.link) {
> +			mode = container_of(clone_output->base.current_mode,
> +					    struct drm_mode, base);
> +
> +			ret = drmModeSetCrtc(compositor->drm.fd,
> +					     clone_output->crtc_id,
> +					     output->next->fb_id, 0, 0,
> +					     &clone_output->connector_id, 1,
> +					     &mode->mode_info);
> +			if (ret) {
> +				weston_log("set mode failed:%m\n");
> +				goto err_pageflip;
> +			}
> +
> +			clone_output->base.set_dpms(&clone_output->base,
> +						    WESTON_DPMS_ON);
> +		}
>  	}
>  
>  	if (drmModePageFlip(compositor->drm.fd, output->crtc_id,
> @@ -622,6 +643,23 @@ drm_output_repaint(struct weston_output *output_base,
>  	}
>  
>  	output->page_flip_pending = 1;
> +	output->page_flip_count++;
> +
> +	/* clone output repaint*/
> +	wl_list_for_each(clone_output, &output->base.clone_output_list,
> +			 base.link) {
> +		if (drmModePageFlip(compositor->drm.fd,
> +				    clone_output->crtc_id,
> +				    output->next->fb_id,
> +				    DRM_MODE_PAGE_FLIP_EVENT,
> +				    clone_output) < 0) {
> +			weston_log("queueing pageflip failed:%m\n");
> +			goto err_pageflip;
> +		}
> +
> +		clone_output->page_flip_pending = 1;
> +		output->page_flip_count++;
> +	}
>  
>  	drm_output_set_cursor(output);
>  
> @@ -691,8 +729,9 @@ drm_output_start_repaint_loop(struct weston_output *output_base)
>  	uint32_t fb_id;
>  	uint32_t msec;
>  	struct timespec ts;
> +	struct drm_output *clone_output;
>  
> -	if (output->destroy_pending)
> +	if (output->destroy_pending || output->base.is_slave)
>  		return;
>  
>  	if (!output->current) {
> @@ -707,6 +746,22 @@ drm_output_start_repaint_loop(struct weston_output *output_base)
>  		weston_log("queueing pageflip failed: %m\n");
>  		goto finish_frame;
>  	}
> +	output->page_flip_count++;
> +
> +	/* clone output repaint*/
> +	wl_list_for_each(clone_output, &output->base.clone_output_list,
> +			 base.link) {
> +		if (drmModePageFlip(compositor->drm.fd,
> +				    clone_output->crtc_id,
> +				    fb_id,
> +				    DRM_MODE_PAGE_FLIP_EVENT,
> +				    clone_output) < 0) {
> +			weston_log("queueing pageflip failed:%m\n");
> +			goto finish_frame;
> +		}
> +
> +		output->page_flip_count++;
> +	}
>  
>  	return;
>  
> @@ -745,8 +800,22 @@ page_flip_handler(int fd, unsigned int frame,
>  		  unsigned int sec, unsigned int usec, void *data)
>  {
>  	struct drm_output *output = (struct drm_output *) data;
> +	struct drm_output *master_output;
>  	uint32_t msecs;
>  
> +	if (output->base.is_slave) {
> +		master_output = (struct drm_output *)output->base.master_output;
> +		master_output->page_flip_count--;
> +		output->page_flip_pending = 0;
> +		if (output->destroy_pending)
> +			drm_output_destroy(&output->base);
> +		output = master_output;
> +	} else
> +		output->page_flip_count--;
> +
> +	if (output->page_flip_count != 0)
> +		return;
> +
>  	/* We don't set page_flip_pending on start_repaint_loop, in that case
>  	 * we just want to page flip to the current buffer to get an accurate
>  	 * timestamp */
> @@ -990,10 +1059,21 @@ drm_output_set_cursor(struct drm_output *output)
>  	uint32_t buf[64 * 64];
>  	unsigned char *s;
>  	int i, x, y;
> +	struct drm_output *clone_output;
>  
>  	output->cursor_view = NULL;
>  	if (ev == NULL) {
>  		drmModeSetCursor(c->drm.fd, output->crtc_id, 0, 0, 0);
> +
> +		if (!output->base.is_slave) {
> +			wl_list_for_each(clone_output,
> +					 &output->base.clone_output_list,
> +					 base.link) {
> +				drmModeSetCursor(c->drm.fd,
> +						clone_output->crtc_id, 0, 0, 0);
> +			}
> +		}
> +
>  		return;
>  	}
>  
> @@ -1023,6 +1103,19 @@ drm_output_set_cursor(struct drm_output *output)
>  			weston_log("failed to set cursor: %m\n");
>  			c->cursors_are_broken = 1;
>  		}
> +
> +		if (!output->base.is_slave) {
> +			wl_list_for_each(clone_output,
> +					 &output->base.clone_output_list,
> +					 base.link) {
> +				if (drmModeSetCursor(c->drm.fd,
> +						     clone_output->crtc_id,
> +						     handle, 64, 64)) {
> +					weston_log("failed to set cursor: %m\n");
> +					c->cursors_are_broken = 1;
> +				}
> +			}
> +		}
>  	}
>  
>  	x = (ev->geometry.x - output->base.x) * output->base.current_scale;
> @@ -1035,6 +1128,21 @@ drm_output_set_cursor(struct drm_output *output)
>  
>  		output->cursor_plane.x = x;
>  		output->cursor_plane.y = y;
> +
> +		if (!output->base.is_slave) {
> +			wl_list_for_each(clone_output,
> +					 &output->base.clone_output_list,
> +					 base.link) {
> +				if (drmModeMoveCursor(c->drm.fd,
> +						clone_output->crtc_id, x, y)) {
> +					weston_log("failed to move cursor: %m\n");
> +					c->cursors_are_broken = 1;
> +				}
> +
> +				clone_output->cursor_plane.x = x;
> +				clone_output->cursor_plane.y = y;
> +			}
> +		}
>  	}
>  }
>  
> diff --git a/src/compositor.c b/src/compositor.c
> index 942c398..2c30334 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3276,7 +3276,8 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c,
>  	weston_output_init_zoom(output);
>  
>  	weston_output_init_geometry(output, x, y);
> -	weston_output_damage(output);
> +	if (!output->is_slave)
> +		weston_output_damage(output);
>  
>  	wl_signal_init(&output->frame_signal);
>  	wl_signal_init(&output->destroy_signal);



More information about the wayland-devel mailing list