Hi,

On Tue, Feb 25, 2020 at 03:09:37PM -0800, Tony Lindgren wrote:
> * Sebastian Reichel <sre@kernel.org> [200225 23:03]:
> > Hi,
> > 
> > On Tue, Feb 25, 2020 at 07:42:37AM -0800, Tony Lindgren wrote:
> > > * Sebastian Reichel <sre@kernel.org> [200225 02:29]:
> > > > On Mon, Feb 24, 2020 at 04:10:11PM -0800, Tony Lindgren wrote:
> > > > > BTW, I think there's also some refcount issue in general where
> > > > > the omapdrm related modules cannot be unloaded any longer?
> > > > 
> > > > I wouldn't be too surprised. The dependencies are quite interesting
> > > > at the moment with omapdss registering omapdrm and then omapdrm
> > > > registers the drm_device, which references the encoders from
> > > > omapdss. I think this is something to look at once Laurent's and
> > > > my branch have been merged to avoid increasing the complexity.
> > > > Technically it should be possible to link everything into one
> > > > module.
> > > 
> > > Well the DSS is really DOSSI for Display Output SubSystem
> > > Interconnect :) The devices on the interconnect are mostly
> > > independent and ideally the toplevel dss driver would just
> > > provide Linux generic resources to dispc and various output
> > > drivers. So probably not a good idea to try to build it all
> > > into a single module.
> > 
> > All the output drivers and dispc are already in a single module:
> > omapdss.ko. There is omapdss-base.ko, omapdss.ko and omapdrm.ko
> > module. omapdss-base.ko contains a few helpers, omapdss.ko contains
> > dispc and all output encoders, omapdrm has the tiler code and
> > wraps some of the custom DSS APIs to DRM APIs. I think the best
> > way forward is to eliminate the custom API and use common DRM
> > APIs directly. Then merge all 3 modules into one module.
> > 
> > In theory one could add modules for each encoder, but practically
> > this only increases complexity. DRM cannnot hotplug encoders, so
> > removing any module results in complete loss of DRM. Also during
> > probe we need to load all modules, since something might be
> > connected. So having extra modules is not really useful?
> 
> Well my main concern here is that we should use generic
> Linux frameworks between the devices within DSS where possible.

Ack.

> I can see a single driver pile of code quickly turn into a
> spaghetti of internal calls instead.

I was only talking about generating a single module, it contains
multiple drivers. And we already have the spaghetti of internal
calls between omapdrm and omapdss. With the modules it only means,
that functions are either exported or accessed via callbacks.

> Also each devices on the DSS interconnect needs to do
> pm_runtime_get for it's struct device naturally.

Sure, but you are talking about things that are already part of a
single module (omapdss.ko). omapdss-base.ko and omapdrm.ko do not
directly access hardware.

> If you can avoid the issues above, then I have no objections
> of just having one module.

Well for now let's get Laurent's and my series forward. I think
the next step would be to get rid of omap_encoder by moving the
encoder init into the DSS output code. Technically we are already
merging omapdrm and omapdss, e.g. omap_connector is gone from
omapdrm after the series.

-- Sebastian