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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux