From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH libdrm 2/3] xf86drm: Add USB support Date: Mon, 2 Jan 2017 14:26:26 +0100 Message-ID: <20170102132626.GA7587@ulmo.ba.sec> References: <20161223174917.29267-1-thierry.reding@gmail.com> <20161223174917.29267-3-thierry.reding@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0805156923==" Return-path: Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 460FF89453 for ; Mon, 2 Jan 2017 13:26:31 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id u144so82071137wmu.0 for ; Mon, 02 Jan 2017 05:26:31 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Emil Velikov Cc: ML dri-devel List-Id: dri-devel@lists.freedesktop.org --===============0805156923== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="vkogqOf2sHV7VnPd" Content-Disposition: inline --vkogqOf2sHV7VnPd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Dec 24, 2016 at 04:38:04PM +0000, Emil Velikov wrote: > Hi Thierry, >=20 > On 23 December 2016 at 17:49, Thierry Reding w= rote: > > Allow DRM/KMS devices hosted on USB to be detected by the drmDevice > > infrastructure. > > > > Signed-off-by: Thierry Reding > > --- > > Note that this is completely untested because I don't have a UDL device > > for testing. I'm fairly confident that this will work, though, and it'd > > be nice to include it before the new platform and host1x busses because > > support for it existed in the kernel longer than for platform devices. > > > Functionality looks spot on, but I'm a bit hesitant to get this in > without any testing. > Can we please extend tests/drmdevice.c (separate patch?) as poke > someone on dri-devel/xorg-devel to give it a quick run ? I can do that. > > +static int drmParseUsbDeviceInfo(int maj, int min, drmUsbDeviceInfoPtr= info) > > +{ > > + char path[PATH_MAX + 1], *value; > > + unsigned int vendor, product; > > + int ret; > > + > > + snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device", maj, min); > > + > > + value =3D sysfs_uevent_get(path, "PRODUCT"); > > + ret =3D sscanf(value, "%x/%x", &vendor, &product); > > + free(value); > > + > > + if (ret <=3D 0) > > + return -errno; > > + > > + info->vendor =3D vendor; > > + info->product =3D product; > > + > Worth fetching bcdDevice as well ? This is currently only parsing the uevent file, which doesn't have bcdDevice. The only data that isn't currently being parsed is TYPE (bDeviceClass/bdeviceSubClass/bDeviceProtocol), not sure if we'd want that. I could of course read bcdDevice from the bcdDevice file. One thing that I realized the other day, and it's relevant to all of drmDevice, not just USB, is that we don't have a good way of preserving ABI compatibility. With the USB and platform/host1x bus patches we're not running into problems yet, but consider what would happen if we added more bus or device info. The drmDevice.businfo and drmDevice.deviceinfo unions could be growing beyond what they are now, causing applications written against old versions to potentially allocate too little memory. I wonder if that's something we need to care about. As long as we keep the bus and device info fixed after they've been added, we should be safe because the bus type will be unknown to earlier versions and hence cause the code to error out early. Technically we wouldn't be able to ever extend one type of bus or device info, though. Thierry --vkogqOf2sHV7VnPd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAlhqVPwACgkQ3SOs138+ s6EUdw//eZqjDoimFD8c5N/dsrF8k+UbYRSZhgXyyVnes2hoIA+AqJ+0hB26ZZN1 YYOfqCa36hdTpa/B+2fPOKZl9c6op/8wpOMpMmENfRt6lVduoAK2S6/ARwLJWlPr AcGdViBCIsiNGqcGdZAfk4mTnIcj/fs/xseaVa/sBPS3v6EHH2Zlm6mWmQuMA7H/ OMbjwubj/jaPdm3c26xsioHFMkKXcEFRmua598ZmShqOI4JJUhc4xXEtWj8gK49u JtL6A5YByEMEG9Lm6wR86xgHJwlAQDqzvavW7CGTzIp9iPj1ZgyMofQbtr/DpDKS zNFvVgaD3wzubRyQLcEDi/eVXsE4JbMO+9h2+CkYV2X/HJI3z0dJdexEiyuOXVdr Ix7d1YvaP79zrePwqH9QBqEomDp6gPDLyG/vaXs6rmivE4GejzjW5jtDz8ogZPKC SfUQJtwimBdYQx3Oe9A2ITzsY3nmXVP/s0POcJzstsRjTUhaIgVKiC/fpYTk48uk L58cJsTp6VYLWqyTXzidAywQs6GP/xpUp9nI4Ur7H6va6A/8Smw/1m/0zpX7Nk7Q 2WYRreq6v599uC3NuuvZQeUP3XWysgTEmnQLr46xfSaevI2F+8b5kw7PGKrMcBxb hx9/30SK4HaUFs9lBXJvuWUoYTTxs0sOaXk3FNeaoG1odY8z1D0= =cqkG -----END PGP SIGNATURE----- --vkogqOf2sHV7VnPd-- --===============0805156923== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0805156923==--