Re: [PATCH RFC] drm/bridge: panel: Add module_get/but calls to attached panel driver

From: Thierry Reding <thierry.reding@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: airlied@linux.ie, tomi.valkeinen@ti.com,
	dri-devel@lists.freedesktop.org, Jyri Sarha <jsarha@ti.com>,
	laurent.pinchart@ideasonboard.com
Subject: Re: [PATCH RFC] drm/bridge: panel: Add module_get/but calls to attached panel driver
Date: Tue, 20 Feb 2018 11:34:53 +0100	[thread overview]
Message-ID: <20180220103453.GH23425@ulmo> (raw)
In-Reply-To: <20180219225923.GG22199@phenom.ffwll.local>

[-- Attachment #1.1: Type: text/plain, Size: 1898 bytes --]

On Mon, Feb 19, 2018 at 11:59:23PM +0100, Daniel Vetter wrote:
> On Mon, Feb 19, 2018 at 10:06:22PM +0200, Jyri Sarha wrote:
> > Currently there is no way for a master drm driver to protect against an
> > attached panel driver from being unloaded while it is in use. The
> > least we can do is to indicate the usage by incrementing the module
> > reference count.
> > 
> > Signed-off-by: Jyri Sarha <jsarha@ti.com>
> > cc: eric@anholt.net
> > cc: laurent.pinchart@ideasonboard.com
> > ---
> > I do not see any module_get/put code in drm core. Is there is a reason
> > for that?
> > 
> > There is two more alternative places for adding the module_get/put
> > code. One is puting it directly to drm_panel_attach() and
> > drm_panel_detach(). However, if the same module implements both the
> > master drm driver and the panel (like tilcdc does with its
> > tilcdc_panel.c), then attaching the panel will lock the module in for
> > no good reason. Still, this solution should work with drm bridges as I
> > do not see any reason why anybody would implement bridge drivers in
> > the same module with the master drm driver.
> > 
> > The other place to put the code would in the master drm driver. But
> > for handling the situation with bridges would need the device pointer
> > in struct drm_bridge.
> 
> I think this looks like a reasonable place to do this. Looking at the code
> we seem to have a similar issue with the bridge driver itself. I think
> we need to wire through the module owner stuff and add a try_modeul_get to
> of_drm_find_bridge (and any other helper used to find bridge instances).

I disagree. module_get() is only going to protect you from unloading a
module that's in use, but there are other ways to unbind a driver from
the device and cause subsequent mayhem.

struct device_link was "recently" introduced to fix that issue.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel