Re: [PATCH v2] drm: shmobile: Perform initialization/cleanup at probe/remove time

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

On Tuesday 13 Dec 2016 22:01:36 Daniel Vetter wrote:
> On Tue, Dec 13, 2016 at 07:19:15PM +0200, Laurent Pinchart wrote:
> > On Tuesday 13 Dec 2016 18:11:34 Daniel Vetter wrote:
> >> On Tue, Dec 13, 2016 at 02:59:48PM +0200, Laurent Pinchart wrote:
> >>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>> 
> >>> The drm driver .load() operation is prone to race conditions as it
> >>> initializes the driver after registering the device nodes. Its usage
> >>> is deprecated, inline it in the probe function and call
> >>> drm_dev_alloc() and drm_dev_register() explicitly.
> >>> 
> >>> For consistency inline the .unload() handler in the remove function as
> >>> well.
> >>> 
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>> ---
> >>> Changes since v1:
> >>> 
> >>> - Removed manual the drm_connector_register() that caused
> >>>   sysfs-related warnings
> >> 
> >> Hm, what did go boom there? We should catch multiple calls to
> >> drm_connector_register ...
> > 
> > Trying to register the connector before the DRM device is registered makes
> > sysfs unhappy due to the lack of a parent.
> 
> Autch. I think for i915 we're safe though - our trouble is that mst
> hotplug starts to happen before we call drm_dev_register (but it's still
> safe there already), because atm the initial topology detection works out
> to be async. But we already have the drm_device at least initialized,
> which I guess with DT and probe-defer isn't guaranteed.
> 
> Or does this even go boom if you just register a child before the parent?

I think it does, but I haven't investigated this any further. Given that we 
have support for auto-registration of connectors at drm_device registration 
time, and that I don't need to register connectors later, I just went the easy 
way. For connectors that can be registered at any time, something more 
elaborate is likely needed.

> >>>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c |   7 +-
> >>>  drivers/gpu/drm/shmobile/shmob_drm_drv.c  | 206 ++++++++++-------------
> >>>  2 files changed, 104 insertions(+), 109 deletions(-)

-- 
Regards,

Laurent Pinchart