[Spice-devel] [PATCH] Update drm properties correctly

[Jonathon Jongsma]

On Thu, 2014-11-06 at 18:59 -0500, Marc-André Lureau wrote:
> 
> ----- Original Message -----
> > When connector properties got changed, those changes were not being
> > propagated to user-space. This pushes those chagnes up so that e.g. new
> > suggested_x|y properties can be used to help lay out multiple displays
> > properly. This code is based on similar code from the nouveau driver.
> > ---
> >  src/qxl_drmmode.c | 54
> >  +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 53 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/qxl_drmmode.c b/src/qxl_drmmode.c
> > index b7ea7d1..899eefd 100644
> > --- a/src/qxl_drmmode.c
> > +++ b/src/qxl_drmmode.c
> > @@ -640,7 +640,59 @@ drmmode_output_set_property(xf86OutputPtr output, Atom
> > property,
> >  static Bool
> >  drmmode_output_get_property(xf86OutputPtr output, Atom property)
> >  {
> > -    return TRUE;
> > +    drmmode_output_private_ptr drmmode_output = output->driver_private;
> > +    drmmode_ptr drmmode = drmmode_output->drmmode;
> > +    uint32_t value;
> > +    int err, i;
> > +
> > +    if (output->scrn->vtSema) {
> > +	drmModeFreeConnector(drmmode_output->mode_output);
> > +	drmmode_output->mode_output =
> > +	    drmModeGetConnector(drmmode->fd, drmmode_output->output_id);
> > +    }
> > +
> > +    if (!drmmode_output->mode_output)
> > +	return FALSE;
> 
> This line is different, is this a normal condition? If not, shouldn't be a warning?

This patch is based on the code here:
http://cgit.freedesktop.org/nouveau/xf86-video-nouveau/tree/src/drmmode_display.c#n1086

If that is not the latest version of the nouveau driver, let me know.

> 
> > +
> > +    for (i = 0; i < drmmode_output->num_props; i++) {
> > +	int n;
> > +	drmmode_prop_ptr p = &drmmode_output->props[i];
> > +	if (p->atoms[0] != property)
> > +	    continue;
> > +
> > +	for (n = 0; n < drmmode_output->mode_output->count_props; n++) {
> > +	    int id = drmmode_output->mode_output->props[n];
> > +	    if (id == p->mode_prop->prop_id) {
> > +		value = drmmode_output->mode_output->prop_values[n];
> 
> and here, nouveau seems to use the p->index to access the value directly.


Right, the definition of drmmode_prop_rec in nouveau is slightly
different than the one in qxl. In nouveau, the struct contains an index
member, but doesn't in qxl. I thought about refactoring the qxl code to
add an index to the property struct, but iterating the array to find the
index didn't seem very expensive, so I didn't bother. If you think it's
too expensive, I can go back and do it.


> 
> > +		break;
> > +	    }
> > +	}
> > +
> > +	if (p->mode_prop->flags & DRM_MODE_PROP_RANGE) {
> > +	    err = RRChangeOutputProperty(output->randr_output,
> > +					 property, XA_INTEGER, 32,
> > +					 PropModeReplace, 1, &value,
> > +					 FALSE, FALSE);
> > +
> > +	    return !err;
> > +	} else if (p->mode_prop->flags & DRM_MODE_PROP_ENUM) {
> > +	    int     j;
> > +
> > +	    /* search for matching name string, then set its value down */
> > +	    for (j = 0; j < p->mode_prop->count_enums; j++) {
> > +		if (p->mode_prop->enums[j].value == value)
> > +		    break;
> > +	    }
> > +
> > +	    err = RRChangeOutputProperty(output->randr_output, property,
> > +					 XA_ATOM, 32, PropModeReplace, 1,
> > +					 &p->atoms[j+1], FALSE, FALSE);
> > +
> > +	    return !err;
> > +	}
> > +    }
> > +
> > +    return FALSE;
> >  }
> 
> looks ok otherwise
> 
> >  
> >  static const xf86OutputFuncsRec drmmode_output_funcs = {
> > --
> > 1.9.3
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >