[PATCH weston] compositor-drm: rename outputs to follow kernel style
Bryce Harrington
bryce at osg.samsung.com
Sat Aug 22 00:00:58 PDT 2015
On Fri, Aug 21, 2015 at 10:35:34AM +0300, Pekka Paalanen wrote:
> On Thu, 20 Aug 2015 11:05:39 -0700
> Bryce Harrington <bryce at osg.samsung.com> wrote:
>
> > On Thu, Aug 20, 2015 at 10:04:45AM +0300, Pekka Paalanen wrote:
> > > On Wed, 19 Aug 2015 10:46:41 -0500
> > > Derek Foreman <derekf at osg.samsung.com> wrote:
> > >
> > > > Excellent!
> > > >
> > > > Incidentally, this is also how Enlightenment names its outputs.
> > > >
> > > > Reviewed-by: Derek Foreman <derekf at osg.samsung.com>
> > >
> > > Cheers.
> > >
> > > Any objections to landing this for 1.9-beta? It does force users to fix
> > > their weston.ini if they have any DRM output configurations there.
> >
> > No objections, if it can be landed soonish. It'd be nice if someone
> > could test it out manually just for a sanity check.
>
> My testing was limited to running Weston on DRM manually, and seeing in
> the log that VGA1 is now VGA-1, and LVDS1 is now LVDS-1. That's all I
> have here.
>
> Would indeed be nice to have tested-by's from the bug reporters.
>
> > (I wish we had some tests with coverage of this chunk of code, so I could
> > give my usual plug for adding test cases; if we did, it'd probably
> > pretty easy to add a couple test cases here and give us a bit extra
> > confidence.)
>
> It's a bit tough in this case, I'm not sure if we can affect what
> outputs the kernel exposes, so it'd be tied to the test machine.
No, the way you'd do it is to construct a "mock kernel", and then
whereever the functionality expects data from the kernel (or is making
kernel calls), you abstract that so you can slip your mock in instead.
Sort of the same idea as how we replaced the graphics card / monitor
with synthetic ones in the headless testing.
I know it probably sounds like a pain in the ass (and it is), but if
you're serious about wanting stronger automated testing of Weston, then
that's the sort of stuff that needs done. And it's certainly
understandable to want to put off spending time writing tests, but it's
the same thing with documentation and look how horribly undocumented
Wayland is - unless you document and write tests as you go, it ain't
gonna get done.
Bryce
> Thanks,
> pq
>
> > > > On 19/08/15 07:54 AM, Pekka Paalanen wrote:
> > > > > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > > >
> > > > > The problem with the old table of names is that it contains duplicates.
> > > > > It is possible to end up with multiple outputs with the same name. In
> > > > > that case you cannot write individual configurations for these outputs
> > > > > in weston.ini, because they are matched by the name.
> > > > >
> > > > > Change all names to follow the kernel naming scheme set in
> > > > > drivers/gpu/drm/drm_crtc.c. The snprintf format now follows the kernel
> > > > > style, too. Use the DRM_MODE_CONNECTOR_* macros rather than implicit
> > > > > table ordering.
> > > > >
> > > > > Completely new entries in the table are "Virtual" and "DSI".
> > > > >
> > > > > There should not be any gaps in the macro values, but if there are, deal
> > > > > with a NULL entry.
> > > > >
> > > > > Also change "UNKNOWN" to "UNNAMED" so it's easier to distinguish from
> > > > > "Unknown" by the kernel.
> > > > >
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89361
> > > > > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > > > ---
> > > > > src/compositor-drm.c | 42 +++++++++++++++++++++++-------------------
> > > > > 1 file changed, 23 insertions(+), 19 deletions(-)
> > > > >
> > > > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > > > > index 26f0012..400bc0c 100644
> > > > > --- a/src/compositor-drm.c
> > > > > +++ b/src/compositor-drm.c
> > > > > @@ -1745,34 +1745,38 @@ drm_set_dpms(struct weston_output *output_base, enum dpms_enum level)
> > > > > }
> > > > >
> > > > > static const char * const connector_type_names[] = {
> > > > > - "None",
> > > > > - "VGA",
> > > > > - "DVI",
> > > > > - "DVI",
> > > > > - "DVI",
> > > > > - "Composite",
> > > > > - "TV",
> > > > > - "LVDS",
> > > > > - "CTV",
> > > > > - "DIN",
> > > > > - "DP",
> > > > > - "HDMI",
> > > > > - "HDMI",
> > > > > - "TV",
> > > > > - "eDP",
> > > > > + [DRM_MODE_CONNECTOR_Unknown] = "Unknown",
> > > > > + [DRM_MODE_CONNECTOR_VGA] = "VGA",
> > > > > + [DRM_MODE_CONNECTOR_DVII] = "DVI-I",
> > > > > + [DRM_MODE_CONNECTOR_DVID] = "DVI-D",
> > > > > + [DRM_MODE_CONNECTOR_DVIA] = "DVI-A",
> > > > > + [DRM_MODE_CONNECTOR_Composite] = "Composite",
> > > > > + [DRM_MODE_CONNECTOR_SVIDEO] = "SVIDEO",
> > > > > + [DRM_MODE_CONNECTOR_LVDS] = "LVDS",
> > > > > + [DRM_MODE_CONNECTOR_Component] = "Component",
> > > > > + [DRM_MODE_CONNECTOR_9PinDIN] = "DIN",
> > > > > + [DRM_MODE_CONNECTOR_DisplayPort] = "DP",
> > > > > + [DRM_MODE_CONNECTOR_HDMIA] = "HDMI-A",
> > > > > + [DRM_MODE_CONNECTOR_HDMIB] = "HDMI-B",
> > > > > + [DRM_MODE_CONNECTOR_TV] = "TV",
> > > > > + [DRM_MODE_CONNECTOR_eDP] = "eDP",
> > > > > + [DRM_MODE_CONNECTOR_VIRTUAL] = "Virtual",
> > > > > + [DRM_MODE_CONNECTOR_DSI] = "DSI",
> > > > > };
> > > > >
> > > > > static char *
> > > > > make_connector_name(const drmModeConnector *con)
> > > > > {
> > > > > char name[32];
> > > > > - const char *type_name;
> > > > > + const char *type_name = NULL;
> > > > >
> > > > > if (con->connector_type < ARRAY_LENGTH(connector_type_names))
> > > > > type_name = connector_type_names[con->connector_type];
> > > > > - else
> > > > > - type_name = "UNKNOWN";
> > > > > - snprintf(name, sizeof name, "%s%d", type_name, con->connector_type_id);
> > > > > +
> > > > > + if (!type_name)
> > > > > + type_name = "UNNAMED";
> > > > > +
> > > > > + snprintf(name, sizeof name, "%s-%d", type_name, con->connector_type_id);
> > > > >
> > > > > return strdup(name);
> > > > > }
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIVAwUBVdbUziNf5bQRqqqnAQi/YQ//dUk3zKGI9b23d/Dr/fakkffBesBXt8lK
> hJ1A8UDMDju37XE9ZmB1RXuFsQ1q7JM4Dpv8yPv/MpTU3n0m6jq1pFt3fT0k8Q+r
> f3erXF4jMjZNiHgcq8gWEBatvuRCid0zT2aFaSRQwB+vDsAB3+ox9sG//QPry9Go
> xj3qa6TR2Tf3v3a8eHCgYOJKVd5npOUdJFzMpmBfUyzNGHpM1329ULAe8FjLVjmz
> ttMWYpjWyE7CmjtGI2z6hTHC0qtHHI6TNmhb6VAD1xySMNMlA4K0FXNBKmls9hLd
> LJubPDfa55B5Tm+xr6T+qHA7MxL17WmZfcDX/iSnPDBPU7c+Rfs3eU5CpMcTB6Nm
> xYPrHXw1dyK5vMZff+GUSrnXcHhegF70/dnxqeZ5rPCSrr0WiAIWZYLkcQClONpm
> 3PTt2xPkXLhB04yCxvZ7XOjjP8FPh5RuBH1T3ih1HQ35LgQc97pHA+5/CIwwjXp7
> 8Cv9QLK+voMISYlxxYzliduJvWhpCQ7oOA2AsEQV4c8sfVzXNeVDjma5q1WADrpL
> TCrHX5Is7u7aE/N8X0yxQJyP01BG5kbhc/JKvvZ7mg21IB+OrRp15Bh1LfBL1nPw
> CxGVcQMzrjzXmxWxH8PJXeLKU4TbaSrin/98iAL4RnUkDwoXWbMvNnVX7toDY3BR
> aseM74xPtfU=
> =j7F6
> -----END PGP SIGNATURE-----
More information about the wayland-devel
mailing list