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

[Marc-André Lureau]

Hi

On Fri, Nov 7, 2014 at 5:19 PM, Jonathon Jongsma <jjongsma at redhat.com>
wrote:

> 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.
>

Ok, I was looking at older code. It seems this check was added to "Clean up
some errors on closing" on forced closed. Not sure this will be reached by
QXL driver, but let say it's okay that way.. (I think I would still put a
warning there)


>
> >
> > > +
> > > +    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.
>
>
If it's not too much to add, or you can leave a FIXME in the code I'd say.



> >
> > > +           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
> > >
>
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20141107/ab47c16f/attachment-0001.html>