On 08/20/2012 10:52 AM, InKi Dae wrote:
> 2012/8/20 Joonyoung Shim <jy0922.shim at samsung.com>:
>> 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 <inki.dae at samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
>>> ---
>>> 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
