Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756360AbcC2JNZ (ORCPT ); Tue, 29 Mar 2016 05:13:25 -0400 Received: from us01smtprelay-2.synopsys.com ([198.182.47.9]:40798 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756317AbcC2JNU convert rfc822-to-8bit (ORCPT ); Tue, 29 Mar 2016 05:13:20 -0400 From: Alexey Brodkin To: "daniel@ffwll.ch" CC: "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "dh.herrmann@gmail.com" , "airlied@linux.ie" Subject: Re: [PATCH 2/4 v3] drm: Introduce drm_connector_register_all() helper Thread-Topic: [PATCH 2/4 v3] drm: Introduce drm_connector_register_all() helper Thread-Index: AQHRhOAJs8axIIwlWU6x5Kb1vJqZC59mz4QAgAApQgCACQNngIAADtIA Date: Tue, 29 Mar 2016 09:12:38 +0000 Message-ID: <1459242758.3021.1.camel@synopsys.com> References: <1458722577-20283-1-git-send-email-abrodkin@synopsys.com> <1458722577-20283-3-git-send-email-abrodkin@synopsys.com> <1458740464.3213.9.camel@synopsys.com> <20160329081935.GK2510@phenom.ffwll.local> In-Reply-To: <20160329081935.GK2510@phenom.ffwll.local> Accept-Language: en-US, ru-RU Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.121.8.113] Content-Type: text/plain; charset="utf-7" Content-ID: Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6859 Lines: 109 Hi Daniel, On Tue, 2016-03-29 at 10:19 +-0200, Daniel Vetter wrote: +AD4- On Wed, Mar 23, 2016 at 01:41:05PM +-0000, Alexey Brodkin wrote: +AD4- +AD4- +AD4- +AD4- Hi David, +AD4- +AD4- +AD4- +AD4- On Wed, 2016-03-23 at 12:13 +-0100, David Herrmann wrote: +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- Hi +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- On Wed, Mar 23, 2016 at 9:42 AM, Alexey Brodkin +AD4- +AD4- +AD4- +ADw-Alexey.Brodkin+AEA-synopsys.com+AD4- wrote: +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- As a pair to already existing drm+AF8-connector+AF8-unregister+AF8-all() we're adding +AD4- +AD4- +AD4- +AD4- generic implementation of what is already done in some drivers. +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- Once this helper is implemented we'll be ready to switch existing +AD4- +AD4- +AD4- +AD4- driver-specific implementations with the generic one. +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- Signed-off-by: Alexey Brodkin +ADw-abrodkin+AEA-synopsys.com+AD4- +AD4- +AD4- +AD4- +AD4- Cc: Daniel Vetter +ADw-daniel+AEA-ffwll.ch+AD4- +AD4- +AD4- +AD4- +AD4- Cc: David Airlie +ADw-airlied+AEA-linux.ie+AD4- +AD4- +AD4- +AD4- +AD4- --- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- Changes v2 -+AD4- v3: +AD4- +AD4- +AD4- +AD4- +AKAAKg- Updated title with capital after colon +AD4- +AD4- +AD4- +AD4- +AKAAKg- Simplified failure path with direct and unconditional invocation of +AD4- +AD4- +AD4- +AD4- +AKAAoACg-unregister+AF8-all() +AD4- +AD4- +AD4- +AD4- +AKAAKg- Updated kerneldoc description of the drm+AF8-connector+AF8-register+AF8-all() +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- Changes v1 -+AD4- v2: +AD4- +AD4- +AD4- +AD4- +AKAAKg- Rename drm+AF8-connector+AF8-unplug+AF8-all() to drm+AF8-connector+AF8-unregister+AF8-all() +AD4- +AD4- +AD4- +AD4- +AKAAKg- Use drm+AF8-for+AF8-each+AF8-connector() instead of list+AF8-for+AF8-each+AF8-entry() +AD4- +AD4- +AD4- +AD4- +AKAAKg- Updated kerneldoc for drm+AF8-dev+AF8-register() +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AKA-drivers/gpu/drm/drm+AF8-crtc.c +AHw- 43 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+- +AD4- +AD4- +AD4- +AD4- +AKA-drivers/gpu/drm/drm+AF8-drv.c+AKAAoAB8AKAAoA-6 +-+-+-+-+-- +AD4- +AD4- +AD4- +AD4- +AKA-include/drm/drm+AF8-crtc.h+AKAAoACgAKAAoAB8AKAAoA-3 +-+-- +AD4- +AD4- +AD4- +AD4- +AKA-3 files changed, 50 insertions(+-), 2 deletions(-) +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- diff --git a/drivers/gpu/drm/drm+AF8-crtc.c b/drivers/gpu/drm/drm+AF8-crtc.c +AD4- +AD4- +AD4- +AD4- index 65488a6..21eea11 100644 +AD4- +AD4- +AD4- +AD4- --- a/drivers/gpu/drm/drm+AF8-crtc.c +AD4- +AD4- +AD4- +AD4- +-+-+- b/drivers/gpu/drm/drm+AF8-crtc.c +AD4- +AD4- +AD4- +AD4- +AEAAQA- -1081,6 +-1081,49 +AEAAQA- void drm+AF8-connector+AF8-unregister(struct drm+AF8-connector +ACo-connector) +AD4- +AD4- +AD4- +AD4- +AKA-EXPORT+AF8-SYMBOL(drm+AF8-connector+AF8-unregister)+ADs- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AKA-/+ACoAKg- +AD4- +AD4- +AD4- +AD4- +- +ACo- drm+AF8-connector+AF8-register+AF8-all - register all connectors +AD4- +AD4- +AD4- +AD4- +- +ACo- +AEA-dev: drm device +AD4- +AD4- +AD4- +AD4- +- +ACo- +AD4- +AD4- +AD4- +AD4- +- +ACo- This function registers all connectors in sysfs and other places so that +AD4- +AD4- +AD4- +AD4- +- +ACo- userspace can start to access them. Drivers can call it after calling +AD4- +AD4- +AD4- +AD4- +- +ACo- drm+AF8-dev+AF8-register() to complete the device registration, if they don't call +AD4- +AD4- +AD4- +AD4- +- +ACo- drm+AF8-connector+AF8-register() on each connector individually. +AD4- +AD4- +AD4- +AD4- +- +ACo- +AD4- +AD4- +AD4- +AD4- +- +ACo- When a device is unplugged and should be removed from userspace access, +AD4- +AD4- +AD4- +AD4- +- +ACo- call drm+AF8-connector+AF8-unregister+AF8-all(), which is the inverse of this +AD4- +AD4- +AD4- +AD4- +- +ACo- function. +AD4- +AD4- +AD4- +AD4- +- +ACo- +AD4- +AD4- +AD4- +AD4- +- +ACo- Returns: +AD4- +AD4- +AD4- +AD4- +- +ACo- Zero on success, error code on failure. +AD4- +AD4- +AD4- +AD4- +- +ACo-/ +AD4- +AD4- +AD4- +AD4- +-int drm+AF8-connector+AF8-register+AF8-all(struct drm+AF8-device +ACo-dev) +AD4- +AD4- +AD4- +AD4- +-+AHs- +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-struct drm+AF8-connector +ACo-connector+ADs- +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-int ret+ADs- +AD4- +AD4- +AD4- +AD4- +- +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-mutex+AF8-lock(+ACY-dev-+AD4-mode+AF8-config.mutex)+ADs- +AD4- +AD4- +AD4- +AD4- +- +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-drm+AF8-for+AF8-each+AF8-connector(connector, dev) +AHs- +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACg-ret +AD0- drm+AF8-connector+AF8-register(connector)+ADs- +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACg-if (ret) +AHs- +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoA-/+ACo- +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgACo- We may safely call unregister+AF8-all() here within +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgACo- area locked with mutex because unregister+AF8-all() +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgACo- doesn't use locks inside (see a comment in that +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgACo- function). +AD4- +AD4- +AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACgACo-/ +AD4- +AD4- +AD4- Ugh? unregister+AF8-all() says: +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- /+ACo- FIXME: taking the mode config mutex ends up in a clash with sysfs +ACo-/ +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- This strongly contradicts your comment. Anyway, regardless how you +AD4- +AD4- +AD4- want to fix it: You better unlock the mode-config mutex before +AD4- +AD4- +AD4- returning below. +AD4- +AD4- So good catch. +AD4- +AD4- But what I really meant since we didn't get any further after registering +AD4- +AD4- all +ACI-good+ACI- connections (see we're not releasing mutex still) the will be +AD4- +AD4- no clashes with sysfs. +AD4- +AD4- +AD4- +AD4- Still I+ADs-d like Daniel to comment on that separately. +AD4- I think doing the unregister+AF8-all outside of the loop+ACY-locked section is +AD4- better for future-proofing. My long-term plan for connector lifetimes and +AD4- the connector list is: +AD4- - refcounting for connectors (Dave has wip patches already). +AD4- - separate lock for the connector list (and only that). +AD4- +AD4- Doing it entirely separate would make things easier and more robust. Ok makes sense. +AD4- I merged patch 1 meanwhile to drm-misc. So may I re-send only patches 2-4 then (using +ACI-drm-misc+ACI- as a base)? -Alexey