[PATCH 1/2] compositor-drm: Cache the DPMS property on drm_output

[Daniel Vetter]

On Wed, Jun 05, 2013 at 12:03:37AM -0400, Kristian Høgsberg wrote:
> On Tue, Jun 04, 2013 at 04:24:04PM +0300, Ander Conselvan de Oliveira wrote:
> > From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
> > 
> > This avoids one drmModeGetConnector() call every time the DPMS mode is
> > set. That call can take hundreds of milliseconds due to DDC.
> 
> This looks good.  As I replied to David, I think this is a net
> improvement, I don't see that it regresses our current vt switching
> handling.  I've committed this and 2/2, but if there's something I'm
> missing, let's do a follow up patch.

Just as an aside, if you hand-roll GetConnector you could grab the
kernel's latest output state and mode list without forcing a slow detect
cycle by setting output->num_modes == 0. The kernel uses that to only do
the detect stuff once instead for both ioctl calls ...
-Daniel

> 
> Kristian
> 
> > ---
> >  src/compositor-drm.c |   21 +++++++--------------
> >  1 file changed, 7 insertions(+), 14 deletions(-)
> > 
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 4222e57..27d4d02 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -145,6 +145,7 @@ struct drm_output {
> >  	uint32_t connector_id;
> >  	drmModeCrtcPtr original_crtc;
> >  	struct drm_edid edid;
> > +	drmModePropertyPtr dpms_prop;
> >  
> >  	int vblank_pending;
> >  	int page_flip_pending;
> > @@ -1054,6 +1055,8 @@ drm_output_destroy(struct weston_output *output_base)
> >  	if (output->backlight)
> >  		backlight_destroy(output->backlight);
> >  
> > +	drmModeFreeProperty(output->dpms_prop);
> > +
> >  	/* Turn off hardware cursor */
> >  	drmModeSetCursor(c->drm.fd, output->crtc_id, 0, 0, 0);
> >  
> > @@ -1365,23 +1368,12 @@ drm_set_dpms(struct weston_output *output_base, enum dpms_enum level)
> >  	struct drm_output *output = (struct drm_output *) output_base;
> >  	struct weston_compositor *ec = output_base->compositor;
> >  	struct drm_compositor *c = (struct drm_compositor *) ec;
> > -	drmModeConnectorPtr connector;
> > -	drmModePropertyPtr prop;
> >  
> > -	connector = drmModeGetConnector(c->drm.fd, output->connector_id);
> > -	if (!connector)
> > +	if (!output->dpms_prop)
> >  		return;
> >  
> > -	prop = drm_get_prop(c->drm.fd, connector, "DPMS");
> > -	if (!prop) {
> > -		drmModeFreeConnector(connector);
> > -		return;
> > -	}
> > -
> > -	drmModeConnectorSetProperty(c->drm.fd, connector->connector_id,
> > -				    prop->prop_id, level);
> > -	drmModeFreeProperty(prop);
> > -	drmModeFreeConnector(connector);
> > +	drmModeConnectorSetProperty(c->drm.fd, output->connector_id,
> > +				    output->dpms_prop->prop_id, level);
> >  }
> >  
> >  static const char *connector_type_names[] = {
> > @@ -1813,6 +1805,7 @@ create_output_for_connector(struct drm_compositor *ec,
> >  	ec->connector_allocator |= (1 << output->connector_id);
> >  
> >  	output->original_crtc = drmModeGetCrtc(ec->drm.fd, output->crtc_id);
> > +	output->dpms_prop = drm_get_prop(ec->drm.fd, connector, "DPMS");
> >  
> >  	/* Get the current mode on the crtc that's currently driving
> >  	 * this connector. */
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch