From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joonyoung Shim Subject: Re: [PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation. Date: Mon, 20 Aug 2012 10:59:23 +0900 Message-ID: <503199FB.1020906@samsung.com> References: <1345197059-25583-1-git-send-email-inki.dae@samsung.com> <1345197059-25583-3-git-send-email-inki.dae@samsung.com> <50318EA5.8020505@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mailout1.samsung.com (mailout1.samsung.com [203.254.224.24]) by gabe.freedesktop.org (Postfix) with ESMTP id DAB1B9E9FF for ; Sun, 19 Aug 2012 18:59:22 -0700 (PDT) Received: from epcpsbgm1.samsung.com (mailout1.samsung.com [203.254.224.24]) by mailout1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0M91001726TPLLI0@mailout1.samsung.com> for dri-devel@lists.freedesktop.org; Mon, 20 Aug 2012 10:59:21 +0900 (KST) Received: from [10.90.51.60] by mmp2.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0M91006B76UWQF70@mmp2.samsung.com> for dri-devel@lists.freedesktop.org; Mon, 20 Aug 2012 10:59:20 +0900 (KST) In-reply-to: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: InKi Dae Cc: kyungmin.park@samsung.com, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 08/20/2012 10:52 AM, InKi Dae wrote: > 2012/8/20 Joonyoung Shim : >> On 08/17/2012 06:50 PM, Inki Dae wrote: >>> this patch separates sub driver's probe call and encoder/connector >>> creation >>> so that exynos drm core module can take exception when some operation was >>> failed properly. >> >> Which exceptions? I don't know this patch gives any benefit to us. >> > previous code didn't take exception when exynos_drm_encoder_create() > is falied. No, it is considered. > and for now, exynos_drm_encoder/connector_create functions > was called at exynos_drm_subdrv_probe() so know that this patch is for > code clean by separating them into two parts also. It's ok, but it just splitting. > >>> Signed-off-by: Inki Dae >>> Signed-off-by: Kyungmin Park >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_core.c | 93 >>> +++++++++++++++++++++--------- >>> 1 files changed, 65 insertions(+), 28 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c >>> b/drivers/gpu/drm/exynos/exynos_drm_core.c >>> index 84dd099..1c8d5fe 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_core.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_core.c >>> @@ -34,30 +34,15 @@ >>> static LIST_HEAD(exynos_drm_subdrv_list); >>> -static int exynos_drm_subdrv_probe(struct drm_device *dev, >>> +static int exynos_drm_create_enc_conn(struct drm_device *dev, >>> struct exynos_drm_subdrv *subdrv) >>> { >>> struct drm_encoder *encoder; >>> struct drm_connector *connector; >>> + int ret; >>> DRM_DEBUG_DRIVER("%s\n", __FILE__); >>> - if (subdrv->probe) { >>> - int ret; >>> - >>> - /* >>> - * this probe callback would be called by sub driver >>> - * after setting of all resources to this sub driver, >>> - * such as clock, irq and register map are done or by >>> load() >>> - * of exynos drm driver. >>> - * >>> - * P.S. note that this driver is considered for >>> modularization. >>> - */ >>> - ret = subdrv->probe(dev, subdrv->dev); >>> - if (ret) >>> - return ret; >>> - } >>> - >>> if (!subdrv->manager) >>> return 0; >>> @@ -78,24 +63,22 @@ static int exynos_drm_subdrv_probe(struct drm_device >>> *dev, >>> connector = exynos_drm_connector_create(dev, encoder); >>> if (!connector) { >>> DRM_ERROR("failed to create connector\n"); >>> - encoder->funcs->destroy(encoder); >>> - return -EFAULT; >>> + ret = -EFAULT; >>> + goto err_destroy_encoder; >>> } >>> subdrv->encoder = encoder; >>> subdrv->connector = connector; >>> return 0; >>> + >>> +err_destroy_encoder: >>> + encoder->funcs->destroy(encoder); >>> + return ret; >>> } >>> -static void exynos_drm_subdrv_remove(struct drm_device *dev, >>> - struct exynos_drm_subdrv *subdrv) >>> +static void exynos_drm_destroy_enc_conn(struct exynos_drm_subdrv *subdrv) >>> { >>> - DRM_DEBUG_DRIVER("%s\n", __FILE__); >>> - >>> - if (subdrv->remove) >>> - subdrv->remove(dev); >>> - >>> if (subdrv->encoder) { >>> struct drm_encoder *encoder = subdrv->encoder; >>> encoder->funcs->destroy(encoder); >>> @@ -109,9 +92,48 @@ static void exynos_drm_subdrv_remove(struct drm_device >>> *dev, >>> } >>> } >>> +static int exynos_drm_subdrv_probe(struct drm_device *dev, >>> + struct exynos_drm_subdrv *subdrv) >>> +{ >>> + if (subdrv->probe) { >>> + int ret; >>> + >>> + subdrv->drm_dev = dev; >>> + >>> + /* >>> + * this probe callback would be called by sub driver >>> + * after setting of all resources to this sub driver, >>> + * such as clock, irq and register map are done or by >>> load() >>> + * of exynos drm driver. >>> + * >>> + * P.S. note that this driver is considered for >>> modularization. >>> + */ >>> + ret = subdrv->probe(dev, subdrv->dev); >>> + if (ret) >>> + return ret; >>> + } >>> + >>> + if (!subdrv->manager) >>> + return -EINVAL; >>> + >>> + subdrv->manager->dev = subdrv->dev; >>> + >>> + return 0; >>> +} >>> + >>> +static void exynos_drm_subdrv_remove(struct drm_device *dev, >>> + struct exynos_drm_subdrv *subdrv) >>> +{ >>> + DRM_DEBUG_DRIVER("%s\n", __FILE__); >>> + >>> + if (subdrv->remove) >>> + subdrv->remove(dev, subdrv->dev); >>> +} >>> + >>> int exynos_drm_device_register(struct drm_device *dev) >>> { >>> struct exynos_drm_subdrv *subdrv, *n; >>> + unsigned int fine_cnt = 0; >>> int err; >>> DRM_DEBUG_DRIVER("%s\n", __FILE__); >>> @@ -120,14 +142,27 @@ int exynos_drm_device_register(struct drm_device >>> *dev) >>> return -EINVAL; >>> list_for_each_entry_safe(subdrv, n, &exynos_drm_subdrv_list, list) >>> { >>> - subdrv->drm_dev = dev; >>> err = exynos_drm_subdrv_probe(dev, subdrv); >>> if (err) { >>> DRM_DEBUG("exynos drm subdrv probe failed.\n"); >>> list_del(&subdrv->list); >>> + continue; >>> } >>> + >>> + err = exynos_drm_create_enc_conn(dev, subdrv); >>> + if (err) { >>> + DRM_DEBUG("failed to create encoder and >>> connector.\n"); >>> + exynos_drm_subdrv_remove(dev, subdrv); >>> + list_del(&subdrv->list); >>> + continue; >>> + } >>> + >>> + fine_cnt++; >>> } >>> + if (!fine_cnt) >>> + return -EINVAL; >>> + >>> return 0; >>> } >>> EXPORT_SYMBOL_GPL(exynos_drm_device_register); >>> @@ -143,8 +178,10 @@ int exynos_drm_device_unregister(struct drm_device >>> *dev) >>> return -EINVAL; >>> } >>> - list_for_each_entry(subdrv, &exynos_drm_subdrv_list, list) >>> + list_for_each_entry(subdrv, &exynos_drm_subdrv_list, list) { >>> exynos_drm_subdrv_remove(dev, subdrv); >>> + exynos_drm_destroy_enc_conn(subdrv); >>> + } >>> return 0; >>> } >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel