2012/8/20 Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>:
> On 08/20/2012 01:31 PM, InKi Dae wrote:
>>
>> 2012/8/20 Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>:
>>>
>>> On 08/20/2012 10:52 AM, InKi Dae wrote:
>>>>
>>>> 2012/8/20 Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>:
>>>>>
>>>>> 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 <inki.dae@xxxxxxxxxxx>
>>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
>>>>>> ---
>>>>>> 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@xxxxxxxxxxxxxxxxxxxxx
>>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
