On 08/20/2012 01:31 PM, InKi Dae wrote:
> 2012/8/20 Joonyoung Shim <jy0922.shim at samsung.com>:
>> 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.
>>
> 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 <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
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
