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()
>
>> 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
