[PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
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. 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++; } +
[PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
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. 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 (!f
[PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
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. > > 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_DEB
[PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
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. 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. > >> >> 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 >> *
[PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
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 d
[PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
2012/8/20 Joonyoung Shim : > 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. > where is subdrv->remove() called when exynos_drm_encoder_create() is failed? I think if failed then subdrv->remove() should be called and if exynos_drm_connector_create() is failed then not only encoder->funcs->destory() but also subdrv->remove() > >> 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) + ret
[PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
On 08/20/2012 01:31 PM, InKi Dae wrote: > 2012/8/20 Joonyoung Shim : >> 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. >> > where is subdrv->remove() called when exynos_drm_encoder_create() is > failed? I think if failed then subdrv->remove() should be called and > if exynos_drm_connector_create() is failed then not only > encoder->funcs->destory() but also subdrv->remove() OK, but just add subdrv->remove(). Then if it needs, please split function. >>> 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 > mo
[PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
2012/8/20 Joonyoung Shim : > On 08/20/2012 01:31 PM, InKi Dae wrote: >> >> 2012/8/20 Joonyoung Shim : >>> >>> 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. >>> >> where is subdrv->remove() called when exynos_drm_encoder_create() is >> failed? I think if failed then subdrv->remove() should be called and >> if exynos_drm_connector_create() is failed then not only >> encoder->funcs->destory() but also subdrv->remove() > > > OK, but just add subdrv->remove(). Then if it needs, please split function. > now exynos_drm_subdrv_probe() creates encoder and connector so it should be separated into meaningful parts. > 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 >>
Re: [PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
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. 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(&
Re: [PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
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. 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. > >> >> 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 >> *
Re: [PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
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) {
Re: [PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
2012/8/20 Joonyoung Shim : > 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. > where is subdrv->remove() called when exynos_drm_encoder_create() is failed? I think if failed then subdrv->remove() should be called and if exynos_drm_connector_create() is failed then not only encoder->funcs->destory() but also subdrv->remove() > >> 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) + ret
Re: [PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
On 08/20/2012 01:31 PM, InKi Dae wrote: 2012/8/20 Joonyoung Shim : 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. where is subdrv->remove() called when exynos_drm_encoder_create() is failed? I think if failed then subdrv->remove() should be called and if exynos_drm_connector_create() is failed then not only encoder->funcs->destory() but also subdrv->remove() OK, but just add subdrv->remove(). Then if it needs, please split function. 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;
Re: [PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.
2012/8/20 Joonyoung Shim : > On 08/20/2012 01:31 PM, InKi Dae wrote: >> >> 2012/8/20 Joonyoung Shim : >>> >>> 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. >>> >> where is subdrv->remove() called when exynos_drm_encoder_create() is >> failed? I think if failed then subdrv->remove() should be called and >> if exynos_drm_connector_create() is failed then not only >> encoder->funcs->destory() but also subdrv->remove() > > > OK, but just add subdrv->remove(). Then if it needs, please split function. > now exynos_drm_subdrv_probe() creates encoder and connector so it should be separated into meaningful parts. > 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 >>