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

[Daniel Vetter]

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 at ti.com>
> cc: eric at anholt.net
> cc: laurent.pinchart at 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).

Care to look into that part of the problem, too?
-Daniel
> 
>  drivers/gpu/drm/bridge/panel.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index 6d99d4a..0a10be6 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -161,6 +161,10 @@ struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
>  	if (!panel)
>  		return ERR_PTR(-EINVAL);
>  
> +	if (WARN_ON(!panel->dev->driver) ||
> +	    !try_module_get(panel->dev->driver->owner))
> +		return ERR_PTR(-ENODEV);
> +
>  	panel_bridge = devm_kzalloc(panel->dev, sizeof(*panel_bridge),
>  				    GFP_KERNEL);
>  	if (!panel_bridge)
> @@ -199,6 +203,9 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge)
>  	panel_bridge = drm_bridge_to_panel_bridge(bridge);
>  
>  	drm_bridge_remove(bridge);
> +
> +	module_put(panel_bridge->panel->dev->driver->owner);
> +
>  	devm_kfree(panel_bridge->panel->dev, bridge);
>  }
>  EXPORT_SYMBOL(drm_panel_bridge_remove);
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch