Re: [PATCH v2 0/7] drm/tegra: Convert to master/component framework

[Thierry Reding]

On Fri, May 23, 2014 at 08:03:00PM +0900, Greg Kroah-Hartman wrote:
> On Thu, May 22, 2014 at 11:56:23AM +0200, Thierry Reding wrote:
> > On Tue, May 13, 2014 at 05:30:43PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding <treding@xxxxxxxxxx>
> > > 
> > > Hi,
> > > 
> > > This series converts the Tegra DRM driver to the master/component
> > > framework. The length of the series and the list of people in Cc is
> > > mostly due to the fact that Tegra has some special requirements as
> > > opposed to other drivers and therefore requires some changes outside
> > > of DRM.
> > > 
> > > Patch 1 introduces a helper function that can be used by new DRM drivers
> > > that don't rely on legacy userspace ABIs. It allows them to register DRM
> > > devices much more easily and without much of the midlayer that currently
> > > exists in the DRM subsystem.
> > > 
> > > Patches 2 and 3 make some changes to the master/component framework
> > > which are necessary to convert Tegra DRM to use it. All current users of
> > > the API have been converted as part of this patch. Note that at the same
> > > time the drivers are converted to no longer use drm_platform_init() in
> > > favour of the drm_set_unique() from patch 1 in conjunction with
> > > drm_dev_alloc() and drm_dev_register(). I would've liked to avoid such
> > > invasive changes in a single patch, but unfortunately I couldn't think
> > > of a way how to do that. I'm open to suggestions.
> > > 
> > > Patch 4 adds a new interface framework that supplements the master/
> > > component framework and can be used in situations where there is no
> > > struct device * that a driver can bind to.
> > > 
> > > The Tegra DRM driver is converted to using the master/component
> > > framework in patch 5 using the above four patches.
> > > 
> > > Finally patches 6 and 7 add some documentation about the new way of
> > > registering DRM devices that don't need legacy support.
> > > 
> > > Each patch has a somewhat more elaborate description of why it is needed
> > > or what problem it solves. The patchset applies on top of linux-next.
> > > 
> > > I welcome any questions or comments you might have.
> > > 
> > > Thierry
> > > 
> > > Thierry Reding (7):
> > >   drm: Introduce drm_set_unique()
> > >   drivers/base: Allow driver-data to be attached to a master
> > >   drivers/base: Allow multiple masters per device
> > >   drivers/base: Add interface framework
> > >   drm/tegra: Convert to master/component framework
> > >   drm: Add device registration documentation
> > >   drm: Document how to register devices without struct drm_bus
> > 
> > Greg, Russell,
> > 
> > Any comments on this? I've been blocking a bunch of patches in the hopes
> > of getting this merged since it allows the Tegra DRM driver to be
> > cleaned up a great deal, but if nobody's going to look at or comment on
> > this I'll abandon this series and keep using the custom solution we've
> > had for a while now.
> 
> I'd like Russell to comment on this, as he just added some driver core
> work to do drm device work that seems to me to be much the same here.

Yes. In fact that's what this series is all about. It converts the Tegra
DRM driver to use Russell's master/component helpers. But there are also
special requirements on Tegra DRM that require some enhancements to the
helpers, which this series does. Furthermore the interface framework
introduced here is supposed to complement the master/components helpers.
It is not a duplicate implementation but attempts to solves a completely
different problem.

> There's also the work that the media developers are working on to handle
> power management issues that seem to also be hitting the same area.  Due
> to all of that, I really don't want to add yet-another-driver-core
> addition for just one drm driver, all of them should be able to use it.

Like I said, this "interface" framework is meant to address something
different and was specifically designed to be useful for more than just
one DRM driver. However, people seem to prefer to make assumptions about
this code rather than actually go look (or even read the description)
what it really is about.

Besides I already have a working solution for the "one DRM driver". And
replacing it with the component/master helpers as-is will make it worse
than it is. Hence in order to avoid wasting more of everyone's time I
have now dropped patches 2-4 and changed patch 5 to only convert to the
drm_dev_set_unique() function, which is the more important piece of the
series anyway.

Thierry

Attachment: pgpQ_2b5BWY8w.pgp
Description: PGP signature