diff mbox

[02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation.

Message ID 1345197059-25583-3-git-send-email-inki.dae@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Inki Dae Aug. 17, 2012, 9:50 a.m. UTC
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 <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_core.c |   93 +++++++++++++++++++++---------
 1 files changed, 65 insertions(+), 28 deletions(-)

Comments

Joonyoung Shim Aug. 20, 2012, 1:11 a.m. UTC | #1
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 <inki.dae@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@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;
>   }
Inki Dae Aug. 20, 2012, 1:52 a.m. UTC | #2
2012/8/20 Joonyoung Shim <jy0922.shim@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. 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 <inki.dae@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Joonyoung Shim Aug. 20, 2012, 1:59 a.m. UTC | #3
On 08/20/2012 10:52 AM, InKi Dae wrote:
> 2012/8/20 Joonyoung Shim <jy0922.shim@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@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Inki Dae Aug. 20, 2012, 4:31 a.m. UTC | #4
2012/8/20 Joonyoung Shim <jy0922.shim@samsung.com>:
> On 08/20/2012 10:52 AM, InKi Dae wrote:
>>
>> 2012/8/20 Joonyoung Shim <jy0922.shim@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()

>
>> 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@samsung.com>
>>>> Signed-off-by: Kyungmin Park <kyungmin.park@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@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Joonyoung Shim Aug. 20, 2012, 4:42 a.m. UTC | #5
On 08/20/2012 01:31 PM, InKi Dae wrote:
> 2012/8/20 Joonyoung Shim <jy0922.shim@samsung.com>:
>> On 08/20/2012 10:52 AM, InKi Dae wrote:
>>> 2012/8/20 Joonyoung Shim <jy0922.shim@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@samsung.com>
>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@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@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Inki Dae Aug. 20, 2012, 5:43 a.m. UTC | #6
2012/8/20 Joonyoung Shim <jy0922.shim@samsung.com>:
> On 08/20/2012 01:31 PM, InKi Dae wrote:
>>
>> 2012/8/20 Joonyoung Shim <jy0922.shim@samsung.com>:
>>>
>>> On 08/20/2012 10:52 AM, InKi Dae wrote:
>>>>
>>>> 2012/8/20 Joonyoung Shim <jy0922.shim@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.
>

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@samsung.com>
>>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@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@lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

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;
 }