From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH libdrm 3/3] xf86drm: Add platform and host1x bus support Date: Mon, 2 Jan 2017 14:53:09 +0100 Message-ID: <20170102135309.GB7587@ulmo.ba.sec> References: <20161223174917.29267-1-thierry.reding@gmail.com> <20161223174917.29267-4-thierry.reding@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0799962498==" Return-path: Received: from mail-wj0-x243.google.com (mail-wj0-x243.google.com [IPv6:2a00:1450:400c:c01::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 59FD689FCC for ; Mon, 2 Jan 2017 13:53:13 +0000 (UTC) Received: by mail-wj0-x243.google.com with SMTP id kp2so68571331wjc.0 for ; Mon, 02 Jan 2017 05:53:13 -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 --===============0799962498== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lEGEL1/lMxI0MVQ2" Content-Disposition: inline --lEGEL1/lMxI0MVQ2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Dec 24, 2016 at 05:00:27PM +0000, Emil Velikov wrote: > On 23 December 2016 at 17:49, Thierry Reding w= rote: > > From: Thierry Reding > > > > ARM SoCs usually have their DRM/KMS devices on the platform bus, so add > > support for that to enable these devices to be used with the drmDevice > > infrastructure. > > > > NVIDIA Tegra SoCs have an additional level in the hierarchy and DRM/KMS > > devices can also be on the host1x bus. This is mostly equivalent to the > > platform bus. > > > > Signed-off-by: Thierry Reding > > --- > > Note that we could probably get away with treating host1x as platform > > bus. However, they are technically two different busses in the kernel > > and hence we may want to make use of that differentiation later on. > > > Admittedly there's no clear cut either way, but I'm inclined to have > host1x as platform. > I'm leaning towards that since there is no /sys/bus/host1x (afaict), Actually there is: # find /sys/bus/host1x /sys/bus/host1x /sys/bus/host1x/drivers_probe /sys/bus/host1x/devices /sys/bus/host1x/devices/drm /sys/bus/host1x/uevent /sys/bus/host1x/drivers /sys/bus/host1x/drivers/drm /sys/bus/host1x/drivers/drm/bind /sys/bus/host1x/drivers/drm/unbind /sys/bus/host1x/drivers/drm/module /sys/bus/host1x/drivers/drm/uevent /sys/bus/host1x/drivers/drm/drm /sys/bus/host1x/drivers_autoprobe > plus otherwise devices (like imx-vpu/vc4) will end with their own bus > type. I don't think that would happen. Tegra is somewhat special in this case because of the host1x device that is a parent for both display and capture devices. The host1x bus is architected to allow binding each of the DRM/KMS and V4L2 devices (the V4L2 driver hasn't been merged yet) to the host1x device. The reason for this is mostly historical. Back when Tegra support was added there was a strict rule against adding dummy device nodes to the device tree. For Tegra this wasn't much of a problem because the host1x device is the logical choice for a parent, so I just had to write some glue (the host1x bus) to instantiate a dummy device for each composite driver. Later on the component/master infrastructure was added and the rules regarding dummy device nodes were relaxed, so as far as I know all other devices end up anchored to a dummy device on the platform bus. > > +static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInf= oPtr info) > > +{ > > + char path[PATH_MAX + 1], *name; > > + > > + snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device", maj, min); > > + > > + name =3D sysfs_uevent_get(path, "OF_FULLNAME"); > > + strcpy(info->fullname, name); > > + free(name); > > + > Let's be extra careful and ensure that we don't overrun (and properly > terminate) the fixed size fullname[]. Okay, will do. > > + return 0; > > +} > > + > > +static int drmParsePlatformDeviceInfo(int maj, int min, > > + drmPlatformDeviceInfoPtr info) > > +{ > > + /* XXX fill in platform device info */ > > + > Not 100% sure what exactly we should consider bus and what device > info, either way an empty struct is not going to go well. > As a food for thought, here is some info for imx/etna and vc4. >=20 > cat /sys/dev/char/226\:*/device/uevent >=20 > DRIVER=3Detnaviv > OF_NAME=3Dgpu-subsystem > OF_FULLNAME=3D/gpu-subsystem > OF_COMPATIBLE_0=3Dfsl,imx-gpu-subsystem > OF_COMPATIBLE_N=3D1 > MODALIAS=3Dof:Ngpu-subsystemTCfsl,imx-gpu-subsystem > DRIVER=3Dimx-drm > OF_NAME=3Ddisplay-subsystem > OF_FULLNAME=3D/display-subsystem > OF_COMPATIBLE_0=3Dfsl,imx-display-subsystem > OF_COMPATIBLE_N=3D1 >=20 > DRIVER=3Dvc4-drm > OF_NAME=3Dgpu > OF_FULLNAME=3D/soc/gpu > OF_COMPATIBLE_0=3Dbrcm,bcm2835-vc4 > OF_COMPATIBLE_N=3D1 > MODALIAS=3Dof:NgpuTCbrcm,bcm2835-vc4 I think the full name is appropriate for bus info because it's about the closest thing we have to address the device (the equivalent to domain, bus, device and function numbers). The list of compatible strings might be suitable for device information and might be useful for drivers to determine capabilities if the kernel doesn't have another means of communicating that to userspace. Thierry --lEGEL1/lMxI0MVQ2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAlhqW0IACgkQ3SOs138+ s6GGIA/7BGzWhT3wtOAuJpbN4odLGrKrIaFyATkqxbF9fsXum8m3t2MArXMDetYO 9NX4d1iiN4Ay31p9rMLmIp6qS3IrU3NQlWsYl6f/4nM/lkOiclR/rdrAVNnyL/gl HFgzAe6SDZvao9yxsQXlx69q4xGPDodiucAcpBxOBj/sVbq2h6mRz9MJ83CPQnIa GzCoyYmuO/xrvQmHVRL0x+7JuK81+jS2QfF/VktmzOaLXpREMdAblpBcwTzxAzvP 70DfKHsDSfHyJyuZmEV5T4VXiJo59qO+TIocEhuDeLWzklh3g2UN02prIxtQEBK1 N4dKNLEUWf8ufzr4EU3gpKenyIeiP3RNIG8YzyHEqHpvlre2myYIubluvjPBsH36 zMURFiGVhx3jvlFLd7ZnI7bA/t4HYeEywmhYd3xDkNjm58QiAbiQjCt4rW0EBWuC 47CiGue3zhsgBYWJxendjN0p7/QkNdQ0GVcRyuEtF9aEWwLovcERQQ2cmRx6yUPQ oQ/0qlUiXpW4VW0RwH3nS3VNLo+Jze/+8t8zrf+caA51rnK0GwYjT9iHtmKjTVMp rTrsBKGxNTrP5OTG8XiVhk/291hwDJrmKwxwDY3TLH2BEbE6MkD+htgflJiINLB8 cudXP6VpWXsDblhfLyTRHQ9OCCaKY6btv/WzjsFn52oUCzVJ+dQ= =DERj -----END PGP SIGNATURE----- --lEGEL1/lMxI0MVQ2-- --===============0799962498== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0799962498==--