linux-kernel - Re: [RFC v2 6/8] gpu: drm: tegra: Remove redundant host1x

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121205120429.GA29943@avionic-0098.adnet.avionic-design.de>
Date:	Wed, 5 Dec 2012 13:04:29 +0100
From:	Thierry Reding <thierry.reding@...onic-design.de>
To:	Terje Bergström <tbergstrom@...dia.com>
Cc:	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Arto Merilainen <amerilainen@...dia.com>
Subject: Re: [RFC v2 6/8] gpu: drm: tegra: Remove redundant host1x

On Wed, Dec 05, 2012 at 01:47:38PM +0200, Terje Bergström wrote:
> On 05.12.2012 13:13, Thierry Reding wrote:
[...]
> > Oh well, at the time nobody from NVIDIA was involved so I wrote that
> > code in preparation for proper host1x support that I thought I would
> > have to add myself at some point. I'm more than glad that I don't have
> > to do this all by myself. However the patch proposed in this series
> > breaks a number of requirements such as proper encapsulation, which I
> > already mentioned in more detail in another mail.
> 
> Hmm, I'm not sure if I remember that you refer to by the proper
> encapsulation. Is that the fact that we bind DRM to a sub-client?

Yes, but there's more. For instance I went to great lengths to make sure
there's no global data whatsoever. So all the data is bound to the
host1x device in the current code. I know many other drivers like to
take a shortcut and just put these things into global variables but I
didn't want to.

> > The problem that this solves is that the DRM driver needs to be bound to
> > a specific platform device. None of the DRM subdevices are suitable
> > because they are only part of the whole DRM device. I think that host1x
> > is the only device that fits here.
> > 
> > Note that this is only an administrative problem. It shouldn't interfere
> > with the way host1x works. The goal is that the DRM device is registered
> > at the proper hierarchical location.
> > 
> > The circular dependency is indeed a problem, though. Quite frankly I
> > have no idea how to solve this. However the approach taken in the
> > current patch will break several other requirements as I already
> > explained.
> 
> The problem with doing drm_platform_init() with host1x device as
> parameter is that drm_get_platform_dev() will take control of drvdata.
> We'd need to put host1x specific struct host1x pointer to some other
> place and I'm not sure what that place could be.

Not anymore. I submitted a patch so that it no longer does that. The
patch was merged about a month and a half ago.

> You're right in that binding to a sub-device is not a nice way. DRM
> framework just needs a "struct device" to bind to. exynos seems to solve
> this by introducing a virtual device and bind to that. I'm not sure if
> this is the best way, but worth considering?

That was discussed a few months back already and nobody seemed to like
the idea. In fact it was as a result of that discussion that Stephen
brought up the idea to register the DRM driver from a central host1x
driver (it may also have been part of a discussion on IRC, I don't
remember exactly).

At the time I spent some time on a patch that introduced drm_soc_init()
to solve this by creating a dummy struct device and registering the
driver on top of that. But I abandoned it in favour of fixing the DRM
platform support code. The approach also didn't provide for the proper
encapsulation.

Thierry

Content of type "application/pgp-signature" skipped