On 27 August 2015 at 00:30, Rob Herring <robherring2@xxxxxxxxx> wrote: > On Wed, Aug 26, 2015 at 7:09 AM, Thierry Reding > <thierry.reding@xxxxxxxxx> wrote: >> On Wed, Aug 26, 2015 at 01:52:29PM +0200, Daniel Vetter wrote: >>> On Tue, Aug 25, 2015 at 04:42:18PM -0400, Rob Clark wrote: >>> > On Mon, Aug 24, 2015 at 9:47 AM, Rob Herring <robherring2@xxxxxxxxx> wrote: >>> > > On Mon, Aug 17, 2015 at 1:30 PM, Eric Anholt <eric@xxxxxxxxxx> wrote: >>> > >> Stephen Warren <swarren@xxxxxxxxxxxxx> writes: >>> > >> >>> > >>> On 08/12/2015 06:56 PM, Eric Anholt wrote: >>> > >>>> Signed-off-by: Eric Anholt <eric@xxxxxxxxxx> >>> > >>> >>> > >>> This one definitely needs a patch description, since someone might not >>> > >>> know what a VC4 is, and "git log" won't show the text from the binding >>> > >>> doc itself. I'd suggest adding the initial paragraph of the binding doc >>> > >>> as the patch description, or more. >>> > >>> >>> > >>>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt >>> > >> >>> > >>>> +- hvss: List of references to HVS video scalers >>> > >>>> +- encoders: List of references to output encoders (HDMI, SDTV) >>> > >>> >>> > >>> Would it make sense to make all those nodes child node of the vc4 >>> > >>> object. That way, there's no need to have these lists of objects; they >>> > >>> can be automatically built up as the DT is enumerated. I know that e.g. >>> > >>> the NVIDIA Tegra host1x binding works this way, and I think it may have >>> > >>> been inspired by other similar cases. >>> > >> >>> > >> I've looked at tegra, and the component system used by msm appears to be >>> > >> nicer than it. To follow tegra's model, it looks like I need to build >>> > >> this extra bus thing corresponding to host1x that is effectively the >>> > >> drivers/base/component.c code, so that I can get at vc4's structure from >>> > >> the component drivers. >>> > >> >>> > >>> Of course, this is only appropriate if the HW modules really are >>> > >>> logically children of the VC4 HW module. Perhaps they aren't. If they >>> > >>> aren't though, I wonder what this "vc4" module actually represents in HW? >>> > >> >>> > >> It's the subsystem, same as we use a subsystem node for msm, sti, >>> > >> rockchip, imx, and exynos. This appears to be the common model of how >>> > >> the collection of graphics-related components is represented in the DT. >>> > > >>> > > I think most of these bindings are wrong. They are grouped together >>> > > because that is what DRM wants not because that reflects the h/w. So >>> > > convince me this is one block, not that it is what other people do. >>> > >>> > I think, when it comes to more complex driver subsystems (like drm in >>> > particular) we have a bit of mismatch between how things look from the >>> > "pure hw ignoring sw" perspective, and the "how sw and in particular >>> > userspace expects things" perspective. Maybe it is less a problem in >>> > other subsystems, where bindings map to things that are only visible >>> > in the kernel, or well defined devices like uart or sata controller. >>> > But when given the choice, I'm going to err on the side of not >>> > confusing userspace and the large software stack that sits above >>> > drm/kms, over dt purity. >>> > >>> > Maybe it would be nice to have a sort of dt overlay that adds the bits >>> > needed to tie together hw blocks that should be assembled into a >>> > single logical device for linux and userspace (but maybe not some >>> > other hypothetical operating system). But so far that doesn't exist. >>> > All we have is a hammer (devicetree), everything looks like a nail. >>> > End result is we end up adding some things in the bindings which >>> > aren't purely about the hw. Until someone invents a screwdriver, I'm >>> > not sure what else to do. In the end, other hypothetical OS is free >>> > to ignore those extra fields in the bindings if it doesn't need them. >>> > So meh? >>> >>> I thought we agreed a while back that these kind of "pull everything for >>> the logical device together" dt nodes which just have piles of phandles >>> are totally accepted? At least that's the point behind the component >>> helpers, and Eric even suggested to create dt-specific component helpers >>> to cut down a bit on the usual boilerplate. dt maintainers are also fine >>> with this approach afaik. From my understanding tegra with the host1x bus >>> really is the odd one out and not the norm. >> >> I agree that in many aspects Tegra is somewhat special. But the same >> principles that the host1x infrastructure uses could be implemented in a >> similar way for other DRM drivers. You can easily collect information >> about subdevices by walking the device tree and matching on known >> compatible strings. And you can also instantiate the top-level device >> from driver code rather than have it in DT. It should still be possible >> to make this work without an artificial device node in DT. The component >> and master infrastructure is largely orthogonal to that, and as far as I >> remember the only blocker is the need for a top-level device. I wonder >> if perhaps this could be made to work by binding the master to the top- >> level SoC device. >> >> Obviously adding the node in DT is easier, but to my knowledge easy has >> never been a good excuse for mangling DT. Perhaps that's different these >> days... > > I agree we should avoid the virtual node if possible. It is certainly > possible as I started out with one and removed it. At least in my > case, it essentially required the drm_device and crtc to be a single > driver rather than 2 components. However, I'm more concerned that we > are consistent from platform to platform where it makes sense than > whether we have a somewhat questionable node or not. > http://lists.freedesktop.org/archives/dri-devel/2013-July/041159.html So can we at least have some continuity of decision making, bikeshedding this every time we submit a driver isn't giving me any hope going forward. Dave. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
